[FFmpeg-devel] [PATCH] lavf/matroskadec: Demux the PixelCrop* values

[Neil Birkbeck]

On Fri, Apr 15, 2016 at 4:06 AM wm4 <nfxjfg at googlemail.com> wrote:

> On Wed, 6 Apr 2016 17:09:21 -0600
> Nic Wolfe <nic at wolfeden.ca> wrote:
>
> > Thanks for elaborating wm4. Out of curiosity when you say "generic"
> > and "normal" metadata what do you mean? Are you talking about using
> > generic key names or is there another way to store metadata?
> >
> > I have attached a patch which adds a new AVPacketSideDataType and uses
> > that type to store crop values in the AVStream's side data if they're
> > found during Matroska demuxing. I have included a comment in avcodec.h
> > with some of the info Dave shared above but I'm not sure if that is
> > sufficient to call this "well defined".
> >
> > I'm aware that this is a complex topic and this patch is certainly not
> > comprehensive but hopefully it is a step in the right direction.
>
> I think this is much better. The problem are still the semantics. I
> tried to discuss it in Libav, but nothing conclusive has emerged.
>
> Maybe you could send this patch to libav-devel for discussion? (I feel
> a bit sorry for playing this ping pong with a first time contributor.
> If anyone has a better idea how to continue this, please make a
> suggestion.)
>
> By the way, it's not a good idea to send base64 encoded patches. I
> don't think most mail clients can expand them automatically. It
> makes review harder. Just attach them normally. Also, try to avoid
> top-posting.
>

Reviving this thread, as it seems to have the most context on the topic of
PixelCrop in MKV.

I was also investigating adding support for PixelCrop* fields (
https://matroska.org/technical/specs/index.html) for a particular case of
cropping additional padding off the bottom of the image (e.g.,
PixelWidth=1920, PixelHeight=1088, PixelCropBottom=8,
PixelCrop{Top,Right,Left}=0, DisplayWidth=1920, DisplayHeight=1080) which
is required due to a hardware constraint (and codec bitrstream doesn't
support the crop, unlike say h264/hevc). For my use-case, propagating the
MKV fields through to the AVFrame::crop* and silently applying the crop
would be ideal behavior, but I realize that may not generalize to others.

It seems there is still a bunch of complexity around the various edge cases
and interaction with metadata in the container/codec (e.g., which one takes
precedent?).  And the PixelCrop fields also come up in (
https://trac.ffmpeg.org/ticket/7437), where it seems the fields might be a
natural destination for contents of mov/mp4 "clap" atom (
https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFChap3/qtff3.html#//apple_ref/doc/uid/TP40000939-CH205-125850).
This seems partially relevant, as perhaps a common SideData element can
contain the "clap" and  "PixelCrop*" (if possible)--making the interface
more consistent for applications and transmuxing cases

Is an incremental way forward to:
-Support application level support by extract the PixelCrop fields and
exporting as AVStream side data (perhaps works for 'clap' atom too). [say
by revisiting the AVPacketSideData patch in this thread]
-Support transmuxing use case: add support to muxers, mapping between
relevant containers, where/if applicable.
-Support auto-apply decode case. I guess this is close to what autorotate
does. Need to be sure to strip strip metadata so it doesn't get muxed into
output.

<https://trac.ffmpeg.org/ticket/7437>
Any alternatives, or objections, to at least exporting these fields as
stream side data as a first step?


> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>