On Mon, 30 Nov 2015 19:53:41 +0000 Aaron Colwell <acolw...@google.com> wrote:
> On Mon, Nov 30, 2015 at 10:57 AM wm4 <nfx...@googlemail.com> wrote: > > > On Mon, 30 Nov 2015 18:31:56 +0000 > > Aaron Colwell <acolw...@google.com> wrote: > > > > > Hi, > > > Can I get a review for this please? It has been a week since I sent out > > > this patch and I haven't gotten any real feedback about the changes. I've > > > attached a new copy that has been rebased to HEAD. If I need to be doing > > > something different, please let me know so I can make progress on this. > > > > > > Thanks, > > > Aaron > > > > > > On Wed, Nov 25, 2015 at 4:40 PM Aaron Colwell <acolw...@google.com> > > wrote: > > > > > > > On Tue, Nov 24, 2015 at 10:43 AM Kirill Gavrilov <gavr.m...@gmail.com> > > > > wrote: > > > > > > > >> On Mon, Nov 23, 2015 at 11:37 PM, Aaron Colwell <acolw...@google.com> > > > >> wrote: > > > >> > > > >> > matroskaenc.c applies divisors to the display width/height when > > > >> generating > > > >> > stereo content. This patch adds the corresponding multipliers to > > > >> > matroskadec.c > > > >> > so that the original sample aspect ratio can be recovered. > > > >> > > > >> > > > >> Just to link this patch with Matroska Specification notes: > > > >> http://www.matroska.org/technical/specs/notes.html#3D > > > >> > > > >> > The pixel count of the track (PixelWidth/PixelHeight) should be the > > raw > > > >> > amount of pixels (for example 3840x1080 for full HD side by side) > > > >> > and the DisplayWidth/Height in pixels should be the amount of > > pixels for > > > >> > one plane (1920x1080 for that full HD stream). > > > >> > > > > >> > > > > > > > > Is this intended to be a call to action or are you just adding > > information > > > > to this particular thread? I'd like to get the change reviewed so I can > > > > determine whether I need to make any other changes. > > > > > > > > This change basically allows 'ffmpeg -i blah.mkv foo.mkv ; ffprobe > > > > foo.mkv' to show that foo.mkv has the same SAR and DAR as blah.mkv when > > > > blah.mkv contains stereo content. Without this patch the SAR & DAR get > > > > smaller based on whichever dimension is halved by the stereo layout. > > > > > > > > Hope this helps, > > > > Aaron > > > > > > > > > > > > Are you sure matroskadec is wrong and not matroskaenc? > > > > Thanks for the response. > > Yes. I believe matroskadec is wrong. From what I can tell matroskaenc is > writing the correct information to the file. The issue is that matroskadec > does not appear to be doing the right thing to restore the same state that > was present when the file was written. Specifically sample_aspect_ratio > does not get restored to the value it had during writing. > > Here is an example using a SIDE_BY_SIDE_RL file that has > PixelWidthxPixelHeight of "640x360" and a DisplayWidthxDisplayHeight of > "640x360" > > ./ffmpeg -y -i fp3_RL_640x360.webm blah.webm && ./ffprobe blah.webm > > ----[ffmpeg output]---- > Input #0, matroska,webm, from 'fp3_RL_640x360.webm': > Metadata: > encoder : google > Duration: 00:00:04.88, start: 0.000000, bitrate: 208 kb/s > Stream #0:0: Video: vp8, yuv420p, 640x360, SAR 1:1 DAR 16:9, 23.98 fps, > 23.98 tbr, 1k tbn, 1k tbc (default) > Metadata: > stereo_mode : right_left > Side data: > stereo3d: side by side (inverted) > [libvpx-vp9 @ 0x33dcfa0] v1.4.0-729-g8bf791e > Output #0, webm, to 'blah.webm': > Metadata: > encoder : Lavf57.19.100 > Stream #0:0: Video: vp9 (libvpx-vp9), yuv420p, 640x360 [SAR 1:1 DAR > 16:9], q=-1--1, 200 kb/s, 23.98 fps, 1k tbn, 23.98 tbc (default) > Metadata: > stereo_mode : right_left > encoder : Lavc57.16.101 libvpx-vp9 > Side data: > stereo3d: side by side (inverted) > Stream mapping: > Stream #0:0 -> #0:0 (vp8 (native) -> vp9 (libvpx-vp9)) > ----[end of ffmpeg output]---- > > --[ffprobe output]-- > Input #0, matroska,webm, from 'blah.webm': > Metadata: > encoder : Lavf57.19.100 > Duration: 00:00:04.88, start: 0.000000, bitrate: 100 kb/s > Stream #0:0: Video: vp9 (Profile 0), yuv420p(tv), 640x360, SAR 1:2 DAR > 8:9, 23.98 fps, 23.98 tbr, 1k tbn, 1k tbc (default) > Metadata: > stereo_mode : right_left > Side data: > stereo3d: side by side (inverted) > --[end of ffprobe output]-- > > Note that ffprobe SAR and DAR do not match what ffmpeg thought it wrote. > This is what I'm fixing. With my fix, ffprobe reports what ffmpeg said it > wrote. > > Here is what the output looks like with my change: > > ---[ffmpeg output]--- > Input #0, matroska,webm, from 'fp3_RL_640x360.webm': > Metadata: > encoder : google > Duration: 00:00:04.88, start: 0.000000, bitrate: 208 kb/s > Stream #0:0: Video: vp8, yuv420p, 640x360, SAR 2:1 DAR 32:9, 23.98 fps, > 23.98 tbr, 1k tbn, 1k tbc (default) > Metadata: > stereo_mode : right_left > Side data: > stereo3d: side by side (inverted) > [libvpx-vp9 @ 0x1fb0fa0] v1.4.0-729-g8bf791e > Output #0, webm, to 'blah.webm': > Metadata: > encoder : Lavf57.19.100 > Stream #0:0: Video: vp9 (libvpx-vp9), yuv420p, 640x360 [SAR 2:1 DAR > 32:9], q=-1--1, 200 kb/s, 23.98 fps, 1k tbn, 23.98 tbc (default) > Metadata: > stereo_mode : right_left > encoder : Lavc57.16.101 libvpx-vp9 > Side data: > stereo3d: side by side (inverted) > Stream mapping: > Stream #0:0 -> #0:0 (vp8 (native) -> vp9 (libvpx-vp9)) > ---[End of ffmpeg output]--- > > ---[ffprobe output]-- > Input #0, matroska,webm, from 'blah.webm': > Metadata: > encoder : Lavf57.19.100 > Duration: 00:00:04.88, start: 0.000000, bitrate: 100 kb/s > Stream #0:0: Video: vp9 (Profile 0), yuv420p(tv), 640x360, SAR 2:1 DAR > 32:9, 23.98 fps, 23.98 tbr, 1k tbn, 1k tbc (default) > Metadata: > stereo_mode : right_left > Side data: > stereo3d: side by side (inverted) > ---[end of ffprobe output]-- > > Note that ffmpeg & ffprobe now agree on what was written. Also note that > this also causes the SAR to be set properly since PixelWidth == > DisplayWidth in side-by-side mkv content means that you need to scale the > width of each "side" by 2. I believe this is correct, but if I'm mistaken > please help me see where I have gone wrong. OK, now that I've looked again, I agree that this makes sense. > > > I suspect the change to matroskadec would make the behavior > > inconsistent with vf_stereo3d in libavfilter. (But I'm not sure.) > > > > I'm not sure I understand. Do you have a specific example I could try? I'm > not very familiar with that code, but my initial read of it makes me > believe that everything should still work ok because all I'm doing is > making sure the sample_aspect_ratio is properly restored. Something like: ffplay test.mk3d -vf stereo3d=sbs2l:ml I tried your patch, and it actually makes it work better (looks correct with the patch). The patch itself also LGTM. I just wish the Matroska format didn't require such hacks to handle it correctly. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
