| Message ID | 20210823214730.25046-1-jeebjp@gmail.com |
|---|---|
| State | Accepted |
| Commit | 2f0113be3ffb566f1bb7f3140f038318c447da9f |
| Headers | show |
| Series | [FFmpeg-devel,1/2] avcodec/libx264: add support for setting chroma sample location | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Tue, Aug 24, 2021 at 12:47 AM Jan Ekström <jeebjp@gmail.com> wrote: > > --- > libavcodec/libx264.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c > index d48e142e41..379c167e6f 100644 > --- a/libavcodec/libx264.c > +++ b/libavcodec/libx264.c > @@ -870,6 +870,8 @@ static av_cold int X264_init(AVCodecContext *avctx) > x4->params.vui.i_colorprim = avctx->color_primaries; > if (avctx->color_trc != AVCOL_TRC_UNSPECIFIED) > x4->params.vui.i_transfer = avctx->color_trc; > + if (avctx->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED) > + x4->params.vui.i_chroma_loc = avctx->chroma_sample_location - 1; > > if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) > x4->params.b_repeat_headers = 0; > -- > 2.31.1 > I have re-tested these changes (for both x264 and x265), and BT.2100 content that was marked with topleft chroma location now gets that information passed onto the output (whereas previously the content would get interpreted as with the implicit "left" value). Thus, unless there are objections, I will pull this set in soon. The only difference in default would be with unlabeled H.264/HEVC with streams that get that default value ("left") interpreted. Previously, as there was no support for writing the data, those streams would stay without any metadata on the output side, while now, as libavcodec doesn't differentiate between implicit and explicit values, the value gets written into the output, making an implicit value explicit. As far as whether this is a problem, I would consider it a "no", as the decoding result should be the same (due to the implicit default). Best regards, Jan
On Fri, Aug 27, 2021 at 12:58 AM Jan Ekström <jeebjp@gmail.com> wrote: > > On Tue, Aug 24, 2021 at 12:47 AM Jan Ekström <jeebjp@gmail.com> wrote: > > > > --- > > libavcodec/libx264.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c > > index d48e142e41..379c167e6f 100644 > > --- a/libavcodec/libx264.c > > +++ b/libavcodec/libx264.c > > @@ -870,6 +870,8 @@ static av_cold int X264_init(AVCodecContext *avctx) > > x4->params.vui.i_colorprim = avctx->color_primaries; > > if (avctx->color_trc != AVCOL_TRC_UNSPECIFIED) > > x4->params.vui.i_transfer = avctx->color_trc; > > + if (avctx->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED) > > + x4->params.vui.i_chroma_loc = avctx->chroma_sample_location - 1; > > > > if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) > > x4->params.b_repeat_headers = 0; > > -- > > 2.31.1 > > > > I have re-tested these changes (for both x264 and x265), and BT.2100 > content that was marked with topleft chroma location now gets that > information passed onto the output (whereas previously the content > would get interpreted as with the implicit "left" value). Thus, unless > there are objections, I will pull this set in soon. > > The only difference in default would be with unlabeled H.264/HEVC with > streams that get that default value ("left") interpreted. Previously, > as there was no support for writing the data, those streams would stay > without any metadata on the output side, while now, as libavcodec > doesn't differentiate between implicit and explicit values, the value > gets written into the output, making an implicit value explicit. As > far as whether this is a problem, I would consider it a "no", as the > decoding result should be the same (due to the implicit default). Applied the x264 patch as 2f0113be3ffb566f1bb7f3140f038318c447da9f after checking that x264 ignores chroma sample location for non-CHROMA_420 pix_fmts. Will be posting a v2 of the x265 patch as it requires this logic to be in the API client. Jan
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c index d48e142e41..379c167e6f 100644 --- a/libavcodec/libx264.c +++ b/libavcodec/libx264.c @@ -870,6 +870,8 @@ static av_cold int X264_init(AVCodecContext *avctx) x4->params.vui.i_colorprim = avctx->color_primaries; if (avctx->color_trc != AVCOL_TRC_UNSPECIFIED) x4->params.vui.i_transfer = avctx->color_trc; + if (avctx->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED) + x4->params.vui.i_chroma_loc = avctx->chroma_sample_location - 1; if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) x4->params.b_repeat_headers = 0;