Re: [FFmpeg-devel] [PATCH 28/31] vp9_metadata: Improve spec-compliance and warnings
On 09/07/2019 02:10, Andreas Rheinhardt wrote: > The earlier version had three deficits: > 1. It allowed to set the stream to RGB although this is not allowed when > the profile is 0 or 2. > 2. If it set the stream to RGB, then it did not automatically set the > range to full range; the result was that one got a warning every time a > frame with color_config element was processed if the frame originally > had TV range and the user didn't explicitly choose PC range. Now one > gets only one warning in such a situation. > 3. Intra-only frames in profile 0 are automatically BT.601, but if the > user wished another color space, he was not informed about his wishes > being unfulfillable. > > The commit also improves the documentation about this. > > Signed-off-by: Andreas Rheinhardt > --- > doc/bitstream_filters.texi| 8 --- > libavcodec/vp9_metadata_bsf.c | 40 --- > 2 files changed, 33 insertions(+), 15 deletions(-) > > diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi > index a6a5a331f5..47de4c7116 100644 > --- a/doc/bitstream_filters.texi > +++ b/doc/bitstream_filters.texi > @@ -657,7 +657,9 @@ Modify metadata embedded in a VP9 stream. > > @table @option > @item color_space > -Set the color space value in the frame header. > +Set the color space value in the frame header. Note that any frame > +set to RGB will be implicitly set to PC range and that RGB is > +incompatible with profiles 0 and 2. > @table @samp > @item unknown > @item bt601 > @@ -669,8 +671,8 @@ Set the color space value in the frame header. > @end table > > @item color_range > -Set the color range value in the frame header. Note that this cannot > -be set in RGB streams. > +Set the color range value in the frame header. Note that any value > +imposed by the color space will take precedence over this value. > @table @samp > @item tv > @item pc > diff --git a/libavcodec/vp9_metadata_bsf.c b/libavcodec/vp9_metadata_bsf.c > index 1bde1b96aa..52e962b1c0 100644 > --- a/libavcodec/vp9_metadata_bsf.c > +++ b/libavcodec/vp9_metadata_bsf.c > @@ -33,7 +33,7 @@ typedef struct VP9MetadataContext { > int color_space; > int color_range; > > -int color_range_rgb_warned; > +int color_warnings; > } VP9MetadataContext; > > > @@ -56,20 +56,36 @@ static int vp9_metadata_filter(AVBSFContext *bsf, > AVPacket *pkt) > for (i = 0; i < frag->nb_units; i++) { > VP9RawFrame *frame = frag->units[i].content; > VP9RawFrameHeader *header = &frame->header; > +int profile = (header->profile_high_bit << 1) + > header->profile_low_bit; > + > +if (header->frame_type == VP9_KEY_FRAME || > +header->intra_only && profile > 0) { > +if (ctx->color_space >= 0) { > +if (!(profile & 1) && ctx->color_space == VP9_CS_RGB) { > +if (!(ctx->color_warnings & 2)) { > +av_log(bsf, AV_LOG_WARNING, "Warning: RGB " > + "incompatible with profiles 0 and 2.\n"); > +ctx->color_warnings |= 2; > +} > +} else > +header->color_space = ctx->color_space; > +} > > -if (ctx->color_space >= 0) { > -header->color_space = ctx->color_space; > -} > -if (ctx->color_range >= 0) { > -if (ctx->color_range == 0 && > -header->color_space == VP9_CS_RGB && > -!ctx->color_range_rgb_warned) { > -av_log(bsf, AV_LOG_WARNING, "Warning: color_range cannot " > - "be set to limited in RGB streams.\n"); > -ctx->color_range_rgb_warned = 1; > -} else { > +if (ctx->color_range >= 0) > header->color_range = ctx->color_range; > +if (header->color_space == VP9_CS_RGB) { > +if (!(ctx->color_warnings & 1) && !header->color_range) { > +av_log(bsf, AV_LOG_WARNING, "Warning: Color space RGB " > + "implicitly sets color range to PC range.\n"); > +ctx->color_warnings |= 1; > +} > +header->color_range = 1; > } > +} else if (!(ctx->color_warnings & 4) && header->intra_only && > !profile && > + ctx->color_space >= 0 && ctx->color_space != > VP9_CS_BT_601) { > +av_log(bsf, AV_LOG_WARNING, "Warning: Intra-only frames in " > + "profile 0 are automatically BT.601.\n"); > +ctx->color_warnings |= 4; > } > } > > LGTM, applied. Thanks, - Mark ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 28/31] vp9_metadata: Improve spec-compliance and warnings
The earlier version had three deficits: 1. It allowed to set the stream to RGB although this is not allowed when the profile is 0 or 2. 2. If it set the stream to RGB, then it did not automatically set the range to full range; the result was that one got a warning every time a frame with color_config element was processed if the frame originally had TV range and the user didn't explicitly choose PC range. Now one gets only one warning in such a situation. 3. Intra-only frames in profile 0 are automatically BT.601, but if the user wished another color space, he was not informed about his wishes being unfulfillable. The commit also improves the documentation about this. Signed-off-by: Andreas Rheinhardt --- doc/bitstream_filters.texi| 8 --- libavcodec/vp9_metadata_bsf.c | 40 --- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi index a6a5a331f5..47de4c7116 100644 --- a/doc/bitstream_filters.texi +++ b/doc/bitstream_filters.texi @@ -657,7 +657,9 @@ Modify metadata embedded in a VP9 stream. @table @option @item color_space -Set the color space value in the frame header. +Set the color space value in the frame header. Note that any frame +set to RGB will be implicitly set to PC range and that RGB is +incompatible with profiles 0 and 2. @table @samp @item unknown @item bt601 @@ -669,8 +671,8 @@ Set the color space value in the frame header. @end table @item color_range -Set the color range value in the frame header. Note that this cannot -be set in RGB streams. +Set the color range value in the frame header. Note that any value +imposed by the color space will take precedence over this value. @table @samp @item tv @item pc diff --git a/libavcodec/vp9_metadata_bsf.c b/libavcodec/vp9_metadata_bsf.c index 1bde1b96aa..52e962b1c0 100644 --- a/libavcodec/vp9_metadata_bsf.c +++ b/libavcodec/vp9_metadata_bsf.c @@ -33,7 +33,7 @@ typedef struct VP9MetadataContext { int color_space; int color_range; -int color_range_rgb_warned; +int color_warnings; } VP9MetadataContext; @@ -56,20 +56,36 @@ static int vp9_metadata_filter(AVBSFContext *bsf, AVPacket *pkt) for (i = 0; i < frag->nb_units; i++) { VP9RawFrame *frame = frag->units[i].content; VP9RawFrameHeader *header = &frame->header; +int profile = (header->profile_high_bit << 1) + header->profile_low_bit; + +if (header->frame_type == VP9_KEY_FRAME || +header->intra_only && profile > 0) { +if (ctx->color_space >= 0) { +if (!(profile & 1) && ctx->color_space == VP9_CS_RGB) { +if (!(ctx->color_warnings & 2)) { +av_log(bsf, AV_LOG_WARNING, "Warning: RGB " + "incompatible with profiles 0 and 2.\n"); +ctx->color_warnings |= 2; +} +} else +header->color_space = ctx->color_space; +} -if (ctx->color_space >= 0) { -header->color_space = ctx->color_space; -} -if (ctx->color_range >= 0) { -if (ctx->color_range == 0 && -header->color_space == VP9_CS_RGB && -!ctx->color_range_rgb_warned) { -av_log(bsf, AV_LOG_WARNING, "Warning: color_range cannot " - "be set to limited in RGB streams.\n"); -ctx->color_range_rgb_warned = 1; -} else { +if (ctx->color_range >= 0) header->color_range = ctx->color_range; +if (header->color_space == VP9_CS_RGB) { +if (!(ctx->color_warnings & 1) && !header->color_range) { +av_log(bsf, AV_LOG_WARNING, "Warning: Color space RGB " + "implicitly sets color range to PC range.\n"); +ctx->color_warnings |= 1; +} +header->color_range = 1; } +} else if (!(ctx->color_warnings & 4) && header->intra_only && !profile && + ctx->color_space >= 0 && ctx->color_space != VP9_CS_BT_601) { +av_log(bsf, AV_LOG_WARNING, "Warning: Intra-only frames in " + "profile 0 are automatically BT.601.\n"); +ctx->color_warnings |= 4; } } -- 2.21.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 28/31] vp9_metadata: Improve spec-compliance and warnings
The earlier version had three deficits: 1. It allowed to set the stream to RGB although this is not allowed when the profile is 0 or 2. 2. If it set the stream to RGB, then it did not automatically set the range to full range; the result was that one got a warning every time a frame with color_config element was processed if the frame originally had TV range and the user didn't explicitly choose PC range. Now one gets only one warning in such a situation. 3. Intra-only frames in profile 0 are automatically BT.601, but if the user wished another color space, he was not informed about his wishes being unfulfillable. The commit also improves the documentation about this. Signed-off-by: Andreas Rheinhardt --- doc/bitstream_filters.texi| 8 --- libavcodec/vp9_metadata_bsf.c | 40 --- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/doc/bitstream_filters.texi b/doc/bitstream_filters.texi index e5ce7bc0a3..c9dff7ae03 100644 --- a/doc/bitstream_filters.texi +++ b/doc/bitstream_filters.texi @@ -660,7 +660,9 @@ Modify metadata embedded in a VP9 stream. @table @option @item color_space -Set the color space value in the frame header. +Set the color space value in the frame header. Note that any frame +set to RGB will be implicitly set to PC range and that RGB is +incompatible with profiles 0 and 2. @table @samp @item unknown @item bt601 @@ -672,8 +674,8 @@ Set the color space value in the frame header. @end table @item color_range -Set the color range value in the frame header. Note that this cannot -be set in RGB streams. +Set the color range value in the frame header. Note that any value +imposed by the color space will take precedence over this value. @table @samp @item tv @item pc diff --git a/libavcodec/vp9_metadata_bsf.c b/libavcodec/vp9_metadata_bsf.c index 1bde1b96aa..52e962b1c0 100644 --- a/libavcodec/vp9_metadata_bsf.c +++ b/libavcodec/vp9_metadata_bsf.c @@ -33,7 +33,7 @@ typedef struct VP9MetadataContext { int color_space; int color_range; -int color_range_rgb_warned; +int color_warnings; } VP9MetadataContext; @@ -56,20 +56,36 @@ static int vp9_metadata_filter(AVBSFContext *bsf, AVPacket *pkt) for (i = 0; i < frag->nb_units; i++) { VP9RawFrame *frame = frag->units[i].content; VP9RawFrameHeader *header = &frame->header; +int profile = (header->profile_high_bit << 1) + header->profile_low_bit; + +if (header->frame_type == VP9_KEY_FRAME || +header->intra_only && profile > 0) { +if (ctx->color_space >= 0) { +if (!(profile & 1) && ctx->color_space == VP9_CS_RGB) { +if (!(ctx->color_warnings & 2)) { +av_log(bsf, AV_LOG_WARNING, "Warning: RGB " + "incompatible with profiles 0 and 2.\n"); +ctx->color_warnings |= 2; +} +} else +header->color_space = ctx->color_space; +} -if (ctx->color_space >= 0) { -header->color_space = ctx->color_space; -} -if (ctx->color_range >= 0) { -if (ctx->color_range == 0 && -header->color_space == VP9_CS_RGB && -!ctx->color_range_rgb_warned) { -av_log(bsf, AV_LOG_WARNING, "Warning: color_range cannot " - "be set to limited in RGB streams.\n"); -ctx->color_range_rgb_warned = 1; -} else { +if (ctx->color_range >= 0) header->color_range = ctx->color_range; +if (header->color_space == VP9_CS_RGB) { +if (!(ctx->color_warnings & 1) && !header->color_range) { +av_log(bsf, AV_LOG_WARNING, "Warning: Color space RGB " + "implicitly sets color range to PC range.\n"); +ctx->color_warnings |= 1; +} +header->color_range = 1; } +} else if (!(ctx->color_warnings & 4) && header->intra_only && !profile && + ctx->color_space >= 0 && ctx->color_space != VP9_CS_BT_601) { +av_log(bsf, AV_LOG_WARNING, "Warning: Intra-only frames in " + "profile 0 are automatically BT.601.\n"); +ctx->color_warnings |= 4; } } -- 2.21.0 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".