Page MenuHomePhabricator

Add support for "Open with (folder)" as well as directories passed via the command line.
ClosedPublic

Authored by netstar on Apr 22 2017, 4:10 PM.

Details

Reviewers
raster

Diff Detail

Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 4212
Build 4277: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes

"I did this already!" - not you, the commit message!

so still sorting, filtering and doing it async in a thread... (and append results to play list as they come in from thread)... :) ecore_thread_feedback -> hint. :)

netstar updated this revision to Diff 11697.Jun 30 2017, 5:47 AM

Hey, this needs changes and your brain.

netstar updated this revision to Diff 11698.Jun 30 2017, 6:02 AM

Okay, it seems the list is being populated async...my "problem" is the first file loaded is way off from the rest of the list...

Also, I'm sure I'm doing lots of bogus stuff...but the dirs and files are sorted...

netstar updated this revision to Diff 11699.Jun 30 2017, 6:40 AM

Sort directories and files...

netstar updated this revision to Diff 11700.Jun 30 2017, 6:45 AM

Use your brain

netstar updated this revision to Diff 11701.Jun 30 2017, 7:00 AM

Okay this actually works ok...I DONT know what you mean by filtering!!! Based on what? Extension???

netstar updated this revision to Diff 11702.Jun 30 2017, 8:27 AM

Fix BREAKAGE of subtitles!

netstar updated this revision to Diff 11703.Jun 30 2017, 9:46 AM

Too many paramaters

netstar updated this revision to Diff 11704.Jun 30 2017, 10:05 AM

EVerytime I look it at it it annoys me!

So, I *think* the sorting is ok, e.g. i've a folder with shed loads of files not sorted by directory and it sorts that...

netstar updated this revision to Diff 11705.Jun 30 2017, 2:16 PM

LIttle tidy

netstar updated this revision to Diff 11706.Jun 30 2017, 3:41 PM

Use strbuf in main...rename struct..

raster added a comment.Jul 2 2017, 8:34 PM

filtering - yes. i mean like extensions (or globs). like "*.jpg" "*.mp4" etc. or just plain by extension along or by mime type... if you have a dir tree you almost definitely want to limit things added to the play list just to things you CAN play not any and every file found.

raster requested changes to this revision.Jul 2 2017, 10:16 PM

see inline :)

src/bin/main.c
92

this is good. this filtering should be enough for now. while this will work 99% of the time... efreet is unsafe to call from a thread... :/ so you want to move this into the feedback handling function: _cb_feedback_recursion()

117

ok. you send feedback when totally done.... don't do that. imagine a dir tree on a really slow usb1.1 hdd or nfs or whatever filesystem. you may have to sit with a blank rage window with no videos in the play list at all until the entire fs is scanned with everything.

so send it back in chunks. send feedback per directory you scan. AFTER sorting. so:

  1. list.
  2. sort
  3. send feedback

repeat and recurse. :)

This revision now requires changes to proceed.Jul 2 2017, 10:16 PM
netstar updated this revision to Diff 11714.Jul 3 2017, 2:03 AM
netstar edited edge metadata.

Almost done!

Okay, that's actually tidier that way.

Everything is working EXCEPT I'm still setting the win_video_first at the end of the thread.

If I move it elsewhere it borks the ordering!!!

netstar updated this revision to Diff 11719.Jul 3 2017, 3:01 AM

This works

netstar updated this revision to Diff 11721.Jul 3 2017, 3:52 AM

Fix a bug

netstar updated this revision to Diff 11722.Jul 3 2017, 4:03 AM

Play first file found...

netstar updated this revision to Diff 11723.Jul 3 2017, 4:17 AM

Fix subtitles again

netstar updated this revision to Diff 11724.Jul 3 2017, 4:20 AM

Remove unnecessary function

netstar updated this revision to Diff 11727.Jul 3 2017, 10:52 AM

Missed that leak

raster requested changes to this revision.Jul 20 2017, 4:54 AM

see inline :)

src/bin/main.c
22

possibly also make limit of 0 == unlimited? :) and always start the depth at 1? or depth -1 == unlimited? think about it.

63

why is this here. shouldn't the depth++/-- be down in the dirs list walk+free before/after _files_recursion()?

163

any reason to limit to 5 by default? why not be unlimited by default? or well limit to like 99 (just in case of super sillies with recursive symlinks...)

217

probably do range check (>= 0 < N?) certainly >=0 . negatives would not be useful. limit to range?

278

why do this? reasoning? strbuf i mean vs basic snprintf? you know the buf size is already as big as PATH_MAX anyway... so no path will be longer than the buffer anyway?

src/bin/winvid.c
278

one thing. if the video list is visible (mouse over right side of window) ... you need to update the actual widget too as you append more items here... you don't do that. :) imagine scanning every file takes 5 seconds... it will take a long time to populate the list and thus you want the gui visible one to update too.... :)

This revision now requires changes to proceed.Jul 20 2017, 4:54 AM
netstar updated this revision to Diff 11816.Jul 20 2017, 5:37 AM
netstar edited edge metadata.

updatre

netstar marked 5 inline comments as done.Jul 20 2017, 5:39 AM

The remaining is the video list updating the widget. Not too sure how best to do this.

netstar updated this revision to Diff 11817.Jul 20 2017, 5:48 AM

one-line

netstar updated this revision to Diff 11818.Jul 20 2017, 6:00 AM

Start at depth of 1

netstar updated this revision to Diff 11820.Jul 20 2017, 4:43 PM

Add explanation of 0 depth to help and rename definition.

Hey was just looking over this from earlier...I see you're talking about integer underflow with first comment.

Also added clarification in the help re 0 limit...

still more to do. let me know when done :)

Hi @raster

What I missing ???

I'm not always the brightest person!

netstar updated this revision to Diff 11830.Jul 24 2017, 9:47 AM

Update to reflect recent changes

netstar updated this revision to Diff 11865.Jul 28 2017, 2:48 PM

Unused variable

raster requested changes to this revision.Jul 28 2017, 11:17 PM

see what is not done. you dont update the window list with win_video_insert_append() if the video list is up

This revision now requires changes to proceed.Jul 28 2017, 11:17 PM
netstar updated this revision to Diff 11866.Jul 29 2017, 3:50 AM
netstar edited edge metadata.

hide if appending to list of files.

Hey this is the list you made originally, i've tried to answer each point.

  1. possibly also make limit of 0 == unlimited? :) and always start the depth at 1? or depth -1 == unlimited? think about it.
    • DONE.
  2. why is this here. shouldn't the depth++/-- be down in the dirs list walk+free before/after _files_recursion()?
    • DONE - OOPS! :)
  3. any reason to limit to 5 by default? why not be unlimited by default? or well limit to like 99 (just in case of super sillies with recursive symlinks...)
    • DONE - no good reason!!! :)
  4. probably do range check (>= 0 < N?) certainly >=0 . negatives would not be useful. limit to range?
    • DONE - elementary! :)
  5. why do this? reasoning? strbuf i mean vs basic snprintf? you know the buf size is already as big as PATH_MAX anyway... so no path will be longer than the buffer anyway?
    • REMOVED - no good reason :)
  6. one thing. if the video list is visible (mouse over right side of window) ... you need to update the actual widget too as you append more items here... you don't do that. :) imagine scanning every file takes 5 seconds... it will take a long time to populate the list and thus you want the gui visible one to update too.... :)
    • DONE??? also following further comment on win_video_insert_append list will hide.

hmm hiding the list? shouldn't you get the list to append the new files found at the end with actual objects? :)

Hey yes after hiding and adding files and if there is new content:

if (update_content)
  win_list_content_update(recursion->win);

SOrry my messages should be more informative.

raster accepted this revision.Aug 5 2017, 7:13 AM

i had to fix 3 warnings and some formatting... we like to keep formatting consistent so it's easier to read. :)

This revision is now accepted and ready to land.Aug 5 2017, 7:13 AM
netstar updated this revision to Diff 11925.Aug 5 2017, 3:40 PM

thumb: fix optimization (broken free and temp image path).

Use existing extension to save ANOTHER image conversion and save with
tmp file and the extension (fixes evas_object_image_save) which needs
the file name extension to succeed. Then make sure we free after we
move the file. For clarity move the dot to see what we got :)

netstar closed this revision.Aug 5 2017, 3:41 PM

far out added a new patch to an old tree sorry