[FFmpeg-devel] [PATCH] Fix sample_aspect_ratio computation for stereo matroska content.
wm4
nfxjfg at googlemail.com
Tue Dec 1 01:35:32 CET 2015
On Mon, 30 Nov 2015 19:53:41 +0000
Aaron Colwell <acolwell at google.com> wrote:
> On Mon, Nov 30, 2015 at 10:57 AM wm4 <nfxjfg at googlemail.com> wrote:
>
> > On Mon, 30 Nov 2015 18:31:56 +0000
> > Aaron Colwell <acolwell at 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 <acolwell at google.com>
> > wrote:
> > >
> > > > On Tue, Nov 24, 2015 at 10:43 AM Kirill Gavrilov <gavr.mail at gmail.com>
> > > > wrote:
> > > >
> > > >> On Mon, Nov 23, 2015 at 11:37 PM, Aaron Colwell <acolwell at 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.
More information about the ffmpeg-devel
mailing list