New filmstrip #44

whyboris · 2018-12-27T18:34:57Z

⚠️ PR is WIP ⚠️

Thank you @cal2195 for the ffmpeg extraction code to get the images arranged horizontally and resized properly with hstack 👍

I'd love to add black bars between the screens, I suppose 3px would be the best size, do you have any preferences?

I'll try to code it up but might turn to you for help again:
https://video.stackexchange.com/questions/18419/add-black-or-other-color-padding-when-doing-vstack-or-hstack

Cheers!

whyboris · 2018-12-27T18:36:52Z

Thank you for noticing the ; - I typed out the changes you've made on your copy of the branch into my branch to get a better sense of what was happening - but should have been more careful 😅 - noticed it didn't work after the commit and fixed it 👍

Thank you for showing me what to do 🙇

whyboris · 2018-12-27T20:54:27Z

I think it would make the math easier with fewer bugs in the app if we extracted all images onto a standard black background with consistent size regardless of the source video aspect ratio.

I would opt for a 16:9 (~1.777) standard across all screenshots regardless of the underlying resolution. We would then only need to store a single number across the whole .vha file rather than individual widths for each video!

This would require a bit of tricky ffmpeg scripting, but seems like it's doable. We'll start with a black base background. For example, a user selects a height of 100px for all screenshots. We compute the desired output to be 100 * 1.777 = 178 and add 1px for a minimum border on each side (180px total). When extracting screenshots we'll match the height to 100px and keep the video's aspect ratio to output the video to the center of a 180px region (cropping anything that is over 178px wide).

The height (100px) and width (180px) are saved in the .vha file and every thumbnail and filmstrip preview simply uses those dimensions for computing which of the many screen-captures to show.

whyboris · 2018-12-27T20:56:15Z

The upshot of this approach is that within the app we can simply show the relevant region of the horizontal strip (which will have the black bars around each image within the jpg).

whyboris · 2018-12-27T22:09:29Z

I got a script to work, but it works by manually adding vertical stripes (no fun and requires a lot of annoying computation if we do what I described above) ... would prefer to do it another way: pasting into the center of a black rectangle of predetermined size, a (possibly cropped) video screenshot, and attaching the result to the horizontal stack. But for now:

-ss 811.3636363636364 -i 'E:\Movies\someMovie.mp4' \
-ss 1622.7272727272727 -i 'E:\Movies\someMovie.mp4' \
-ss 2434.090909090909 -i 'E:\Movies\someMovie.mp4' \
-ss 3245.4545454545455 -i 'E:\Movies\someMovie.mp4' \
-frames 1 -filter_complex \
"[0:v]scale=300:-2[0]; \
color=black:5x162[0c]; \
[1:v]scale=300:-2[1]; \
color=black:5x162[1c]; \
[2:v]scale=300:-2[2]; \
color=black:5x162[2c]; \
[3:v]scale=300:-2[3]; \
[0][0c][1][1c][2][2c][3]hstack=inputs=7" \
'C:\temp\output.jpg'

ps - I know that my color bars [0c], [1c], etc can be declared as a block rather than in-between frames.

whyboris · 2018-12-27T22:14:29Z

There's a very similar question with a great answer on StackOverflow:
https://superuser.com/questions/547296/resizing-videos-with-ffmpeg-avconv-to-fit-into-static-sized-player

whyboris · 2018-12-27T22:27:00Z

This might be super close to what I want:

-ss 811.3636363636364 -i 'E:\Movies\someMovie.mp4' \
-ss 1622.7272727272727 -i 'E:\Movies\someMovie.mp4' \
-ss 2434.090909090909 -i 'E:\Movies\someMovie.mp4' \
-ss 3245.4545454545455 -i 'E:\Movies\someMovie.mp4' \
-frames 1 -filter_complex \
"[0:v]scale=1000:500:force_original_aspect_ratio=decrease,pad=1000:500:(ow-iw)/2:(oh-ih)/2[0]; \
[1:v]scale=1000:500:force_original_aspect_ratio=decrease,pad=1000:500:(ow-iw)/2:(oh-ih)/2[1]; \
[2:v]scale=1000:500:force_original_aspect_ratio=decrease,pad=1000:500:(ow-iw)/2:(oh-ih)/2[2]; \
[3:v]scale=1000:500:force_original_aspect_ratio=decrease,pad=1000:500:(ow-iw)/2:(oh-ih)/2[3]; \
[0][1][2][3]hstack=inputs=4" \
'C:\temp\ffmpeg\output.jpg'

I'll work on it some more 👍

whyboris · 2018-12-28T00:47:34Z

Branch now works as desired 🎉
I'll now remove previous mentions and needless computations of screenshot widths 👍
Currently there may be rounding errors (1px per scroll) when extracting screenshots not at 300px. Will fix.

whyboris · 2018-12-28T01:33:53Z

Once this PR lands, this issue will be ready to work on: #39

whyboris · 2018-12-28T02:37:48Z

⚠️ Minor bug -- current branch doesn't work if the .vha file name has any spaces in it 😅 - the screenshots don't display because the image path has a space in it
Will fix 👌

cal2195 · 2018-12-28T12:48:44Z

Branch now works as desired 🎉
I'll now remove previous mentions and needless computations of screenshot widths 👍
Currently there may be rounding errors (1px per scroll) when extracting screenshots not at 300px. Will fix.

This is happening for me even at 300px! 😕

cal2195

Looking good! I like the simplicity! 🎉

Added a few comments on my first pass! 👍

I'll test on a more varied video sample soon to confirm everything still works! 💯

cal2195 · 2018-12-28T12:41:12Z

main-support.ts

+
+  // sweet thanks to StackExchange!
+  // https://superuser.com/questions/547296/resizing-videos-with-ffmpeg-avconv-to-fit-into-static-sized-player
+  const fancyScaleFilter = 'scale=' + ratioString + ':force_original_aspect_ratio=decrease,pad=' + ratioString + ':(ow-iw)/2:(oh-ih)/2';


I'm getting an issue when I try to use a thumbnail size of 50px:

grep stderr: Stream mapping: Stream #0:0 (h264) -> scale Stream #1:0 (h264) -> scale Stream #2:0 (h264) -> scale Stream #3:0 (h264) -> scale Stream #4:0 (h264) -> scale Stream #5:0 (h264) -> scale Stream #6:0 (h264) -> scale Stream #7:0 (h264) -> scale Stream #8:0 (h264) -> scale Stream #9:0 (h264) -> scale hstack -> Stream #0:0 (mjpeg) Press [q] to stop, [?] for help grep stderr: [swscaler @ 0x7fd12ada9a00] deprecated pixel format used, make sure you did set range correctly grep stderr: [Parsed_pad_1 @ 0x7fd129099800] Input area 0:0:89:50 not within the padded area 0:0:88:50 or zero-sized [Parsed_pad_1 @ 0x7fd129099800] Failed to configure input pad on Parsed_pad_1 grep stderr: Error reinitializing filters! Failed to inject frame into filter network: Invalid argument grep stderr: Error while processing the decoded data for stream #9:0 grep stderr: Conversion failed!

Input area 0:0:89:50 not within the padded area 0:0:88:50

Looks like rounding error!

main-support.ts

cal2195 · 2018-12-28T12:47:16Z

src/app/components/home/thumbnail/preview.component.html

+    (mousemove)="mouseIsMoving($event)"
+    [ngStyle]="{
+        height: imgHeight + 'px',
+        'background-image': 'url(' + (hover ? fullFilePath : firstFilePath) + ')',


I like this, apart from 2 things:

Can we make it so that the image doesn't flick back to the first image when you stop hovering?

We should sync up the image and video thumbnails (at least the first one) if we're going to do this!

I made the preview flick back to the original image on purpose as requested by a customer. But since the settings menu is cleaner I can add more option toggles -- this one could be it.

I rather like that the video thumbnail image (the -first.jpg) is different from any of the ones in the filmstrip. I'm happy to keep them different.

Yes please! 👍 I like it staying where it is!

Hmm, it conflicts with option 1 though, if it doesn't reappear... maybe it isn't an issue.

I'll put in a settings button that will allow users to choose between retaining the last-viewed thumbnail or to always come back to the first preview image.

What other fixes would you like to see before we can merge this in? (I know there's more polish and bug-fixing to do before this PR is a fully-operational feature, but I'd rather patch them with subsequent PRs rather than spend many more days on this one) 😃

If your happy with bug fixes and changes later - merge away! 🎉 👍

cal2195 · 2018-12-28T14:44:25Z

main-support.ts

@@ -323,8 +333,8 @@ export function takeTenClips(
    concat += '[' + (current - 1) + ':v]' + '[' + (current - 1) + ':a]';
    current++;
  }
-  concat += 'concat=n=' + (totalCount - 1) + ':v=1:a=1';
-  args.push('-filter_complex', concat, saveLocation + '/' + fileHash + '.mp4');
+  concat += 'concat=n=' + (totalCount - 1) + ':v=1:a=1[v][a];[v]scale=' + screenshotHeight + ':-2[v2]';


I just realised, this is now backwards (generates 300px wide, not high)!

Should be -2:screenshotHeight!

I'll also confirm later whether we need -2 or -1!

I copied this change from your other branch without looking into what it does -- feel free to edit anything in takeTenClips method as you see fit 👍

whyboris · 2018-12-28T17:20:24Z

Excellent! Thank you for the approval -- I'll merge this in now and get started on bug-fixing / improvement PR.

🎉

whyboris added 4 commits December 26, 2018 16:16

new filmstrip simpler code

94bea63

Merge branch 'master' into new-filmstrip

f658a03

thank you cal2195 for the horizontal screenshot code

2a0ab49

first draft working

0a8fc9b

whyboris added the ⛑️ WIP Work in progress label Dec 27, 2018

thumbnail preview works with horizontal filmstrip

3aff83a

translate showClips button

398eee3

whyboris added 2 commits December 27, 2018 19:14

filmstrip and thumb correct behavior with correct image extraction

dba1fc7

rename and simplify

b3a09f7

simpler components

d85bbca

whyboris mentioned this pull request Dec 28, 2018

Auto play 10 screenshots on hover option #39

Closed

whyboris requested a review from cal2195 December 28, 2018 01:36

whyboris added ✏️ ready for review PR is ready for review and removed ⛑️ WIP Work in progress labels Dec 28, 2018

proper filmstrip behavior when scrolling to the end

4f9392a

cal2195 requested changes Dec 28, 2018

View reviewed changes

cal2195 approved these changes Dec 28, 2018

View reviewed changes

whyboris merged commit 302a550 into master Dec 28, 2018

whyboris deleted the new-filmstrip branch December 28, 2018 17:21

This was referenced Dec 28, 2018

Filmstrip bugfixes #48

Merged

Horizontal filmstrip bug tracking #50

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New filmstrip #44

New filmstrip #44

whyboris commented Dec 27, 2018

whyboris commented Dec 27, 2018 •

edited

whyboris commented Dec 27, 2018

whyboris commented Dec 27, 2018

whyboris commented Dec 27, 2018 •

edited

whyboris commented Dec 27, 2018

whyboris commented Dec 27, 2018

whyboris commented Dec 28, 2018

whyboris commented Dec 28, 2018

whyboris commented Dec 28, 2018 •

edited

cal2195 commented Dec 28, 2018

cal2195 left a comment

cal2195 Dec 28, 2018

cal2195 Dec 28, 2018

cal2195 Dec 28, 2018

whyboris Dec 28, 2018

cal2195 Dec 28, 2018

whyboris Dec 28, 2018

cal2195 Dec 28, 2018

cal2195 Dec 28, 2018

cal2195 Dec 28, 2018

whyboris Dec 28, 2018

whyboris commented Dec 28, 2018

New filmstrip #44

New filmstrip #44

Conversation

whyboris commented Dec 27, 2018

whyboris commented Dec 27, 2018 • edited

whyboris commented Dec 27, 2018

whyboris commented Dec 27, 2018

whyboris commented Dec 27, 2018 • edited

whyboris commented Dec 27, 2018

whyboris commented Dec 27, 2018

whyboris commented Dec 28, 2018

whyboris commented Dec 28, 2018

whyboris commented Dec 28, 2018 • edited

cal2195 commented Dec 28, 2018

cal2195 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whyboris commented Dec 28, 2018

whyboris commented Dec 27, 2018 •

edited

whyboris commented Dec 27, 2018 •

edited

whyboris commented Dec 28, 2018 •

edited