[FFmpeg-devel] [PATCH] Save FFmpeg colorspace info in openh264 video files.
Gregory J Wolfe
gregory.wolfe at kodakalaris.com
Fri Dec 2 19:12:35 EET 2016
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On
> Behalf Of James Almer
> Sent: Friday, December 02, 2016 9:31 AM
>
> On 12/1/2016 6:26 PM, Gregory J. Wolfe wrote:
> > As of version 1.6, libopenh264 saves (in the output video file)
> > information about the color primaries, transfer characteristics,
> > and color matrix used when the video pixel data was created.
> > This patch sets the required libopenh264 data structures using
> > the FFmpeg colorspace information so that video players will
> > know how to properly decode video files created using FFmpeg
> > and libopenh264.
> >
> > Signed-off-by: Gregory J. Wolfe <gregory.wolfe at kodakalaris.com>
> > ---
> > libavcodec/libopenh264enc.c | 49
> +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
> >
> > diff --git a/libavcodec/libopenh264enc.c
> b/libavcodec/libopenh264enc.c
> > index e84de27..3b019b8 100644
> > --- a/libavcodec/libopenh264enc.c
> > +++ b/libavcodec/libopenh264enc.c
> > @@ -205,6 +205,55 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > }
> > }
> >
> > +#if OPENH264_VER_AT_LEAST(1, 6)
> > + // set video signal type information
> > + param.sSpatialLayers[0].bVideoSignalTypePresent = true;
> > + param.sSpatialLayers[0].uiVideoFormat = VF_UNDEF; // default;
> choices are VF_: COMPONENT, PAL, NTSC, SECAM, MAC, UNDEF
> > + param.sSpatialLayers[0].bFullRange = avctx->color_range ==
> AVCOL_RANGE_JPEG;
> > + param.sSpatialLayers[0].bColorDescriptionPresent = true;
> > + switch (avctx->color_primaries) {
> > + case AVCOL_PRI_BT709:
> param.sSpatialLayers[0].uiColorPrimaries = CP_BT709; break;
> > + case AVCOL_PRI_UNSPECIFIED:
> param.sSpatialLayers[0].uiColorPrimaries = CP_UNDEF; break;
> > + case AVCOL_PRI_BT470M:
> param.sSpatialLayers[0].uiColorPrimaries = CP_BT470M; break;
> > + case AVCOL_PRI_BT470BG:
> param.sSpatialLayers[0].uiColorPrimaries = CP_BT470BG; break;
> > + case AVCOL_PRI_SMPTE170M:
> param.sSpatialLayers[0].uiColorPrimaries = CP_SMPTE170M;
> break;
> > + case AVCOL_PRI_SMPTE240M:
> param.sSpatialLayers[0].uiColorPrimaries = CP_SMPTE240M;
> break;
> > + case AVCOL_PRI_FILM:
> param.sSpatialLayers[0].uiColorPrimaries = CP_FILM; break;
> > + case AVCOL_PRI_BT2020:
> param.sSpatialLayers[0].uiColorPrimaries = CP_BT2020; break;
> > + default: param.sSpatialLayers[0].uiColorPrimaries =
> CP_UNDEF; break;
> > + }
> > + switch (avctx->color_trc) {
> > + case AVCOL_TRC_BT709:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_BT709;
> break;
> > + case AVCOL_TRC_UNSPECIFIED:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_UNDEF;
> break;
> > + case AVCOL_TRC_GAMMA22:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_BT470M;
> break;
> > + case AVCOL_TRC_GAMMA28:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_BT470BG;
> break;
> > + case AVCOL_TRC_SMPTE170M:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_SMPTE170M;
> break;
> > + case AVCOL_TRC_SMPTE240M:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_SMPTE240M;
> break;
> > + case AVCOL_TRC_LINEAR:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_LINEAR;
> break;
> > + case AVCOL_TRC_LOG:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_LOG100;
> break;
> > + case AVCOL_TRC_LOG_SQRT:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_LOG316;
> break;
> > + case AVCOL_TRC_IEC61966_2_4:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_IEC61966_2_4;
> break;
> > + case AVCOL_TRC_BT1361_ECG:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_BT1361E;
> break;
> > + case AVCOL_TRC_IEC61966_2_1:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_IEC61966_2_1;
> break;
> > + case AVCOL_TRC_BT2020_10:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_BT2020_10;
> break;
> > + case AVCOL_TRC_BT2020_12:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_BT2020_12;
> break;
> > + default:
> param.sSpatialLayers[0].uiTransferCharacteristics = TRC_UNDEF;
> break;
> > + }
> > + switch (avctx->colorspace) {
> > + case AVCOL_SPC_RGB:
> param.sSpatialLayers[0].uiColorMatrix = CM_GBR; break;
> > + case AVCOL_SPC_BT709:
> param.sSpatialLayers[0].uiColorMatrix = CM_BT709; break;
> > + case AVCOL_SPC_UNSPECIFIED:
> param.sSpatialLayers[0].uiColorMatrix = CM_UNDEF; break;
> > + case AVCOL_SPC_FCC: param.sSpatialLayers[0].uiColorMatrix
> = CM_FCC; break;
> > + case AVCOL_SPC_BT470BG:
> param.sSpatialLayers[0].uiColorMatrix = CM_BT470BG; break;
> > + case AVCOL_SPC_SMPTE170M:
> param.sSpatialLayers[0].uiColorMatrix = CM_SMPTE170M;
> break;
> > + case AVCOL_SPC_SMPTE240M:
> param.sSpatialLayers[0].uiColorMatrix = CM_SMPTE240M;
> break;
> > + case AVCOL_SPC_YCOCG:
> param.sSpatialLayers[0].uiColorMatrix = CM_YCGCO; break;
> > + case AVCOL_SPC_BT2020_NCL:
> param.sSpatialLayers[0].uiColorMatrix = CM_BT2020NC;
> break;
> > + case AVCOL_SPC_BT2020_CL:
> param.sSpatialLayers[0].uiColorMatrix = CM_BT2020C; break;
> > + default: param.sSpatialLayers[0].uiColorMatrix =
> CM_UNDEF; break;
> > + }
>
> Are you 100% sure these CP_ enums aren't the same as our AVCOL_
> ones?
Verified that these CP_ enums have the same numerical values as
FFmpeg's AVCOL_ ones. However, FFmpeg has additional AVCOL_
enum values that are not in the libopenh264 enums. Same
comments apply to TRC_ and CM_ enums.
> You should be able to simplify this to
>
> param.sSpatialLayers[0].uiColorPrimaries =avctx->color_primaries;
> param.sSpatialLayers[0].uiTransferCharacteristics = avctx->color_trc;
> param.sSpatialLayers[0].uiColorMatrix = avctx->colorspace;
>
> if libopen264 choose to follow the spec values. Which I'd find odd if
> they
> didn't.
>
> And assuming you're filtering values not supported by libopen264,
> shouldn't
> the library simply refuse them and abort encoding at init? That'd be
> IMO
> better than letting the user set for example SMPTE2084 trc and the
> encoder
> silently turning that into undefined.
>
> Note how the colorspace filter refuses values it doesn't support.
As you speculated, the intent here was to filter values not
supported by libopenh264. However, unlike the colorspace
filter, which actually performs computations based on the
colorspace information, this usage in libopenh264 is simply
storing information into the h264 header for later use by
video players (many of which ignore it anyway). Also, note that
it is not an error for the client to specify that the colorspace
information is undefined/unspecified by setting it to CP_UNDEF.
I do agree that it's probably bad to silently turn SMPTE2084 trc
(or any other value not supported by libopenh264) into
"undefined", but not so bad that processing should be aborted.
I propose logging a warning message instead of silently
converting any value to "undefined" that was not actually
specified as "undefined". If this is OK I will update the patch and
resubmit.
One more note. I did notice that support for additional color
primaries was recently added to FFmpeg. Since libopenh264
is simply storing this information into the video file header,
the libopenh264 enums should probably be updated to
include values for these new primaries. The same could
probably be said for the x264 encoder.
Greg W.
More information about the ffmpeg-devel
mailing list