[FFmpeg-devel] [PATCH] lavf/matroskadec: Demux the PixelCrop* values
nfxjfg at googlemail.com
Tue Mar 29 10:23:52 CEST 2016
On Sat, 26 Mar 2016 16:56:55 -0600
Nic Wolfe <nic at wolfeden.ca> wrote:
> The Matroska spec defines PixelCropTop, PixelCropBottom, PixelCropLeft,
> and PixelCropRight elements: https://www.matroska.org/technical/specs/index.html
> This commit adds support for demuxing these values so that
> applications using libav*
> are able to use them when playing the stream. They're added to the AVStream's
> metadata if they are set to something non-zero.
> My official patch is base64 encoded and attached but I will also
> include the diff below for (hopefully) convenience.
To elaborate on why this change is bad (in its current state):
- It's not clearly defined what the pixelcrop fields mean. Do they
operate before or after aspect rasto is applied? Do they affect
aspect ratio calculation? What if aspect ratio or video size change
later? Does it get applied after h264 bitstream cropping, does it
override it? AFAIK these issues were also discussed on the cellar
mailing list, but I didn't follow it.
- There should generally be a concept at least in libavformat's API how
to handle cropping. For example, it could be some sort of well-defined
AVStream side data. (Personally I'd be a fan of adding it to AVFrame
too. There's no way around if it should work for hw decoding.)
- Worst of all: it's exported as generic metadata. This means that:
- API users could start interpreting the same metadata fields for
other formats than Matroska too
- Transcoders like ffmpeg CLI will copy the crop metadata to other
containers (as normal metadata).
- Non-Matroska files might be created that contain the "made
up" libavformat Matroska demuxer metadata, and the creator of the
file expects that programs respect it.
(Something like this almost happened with the "old" libavformat
rotation metadata, which is also exported as normal metadata.)
While just adding a "hack" to export metadata for essentially 1 API
user might be acceptable if adding "proper" API is too hard for now, at
least the last point needs to be fixed.
More information about the ffmpeg-devel