[FFmpeg-devel] [PATCH 1/5] lavc/hevcdec: call export_stream_params_from_sei() before ff_get_buffer()

[FFmpeg-devel] [PATCH 1/5] lavc/hevcdec: call export_stream_params_from_sei() before ff_get_buffer()

[Anton Khirnov]

So that correct values of color_trc are set on the allocated frame.
---
 libavcodec/hevc/hevcdec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index 1d2e53afc3..e80f2f28c7 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -2964,6 +2964,10 @@ static int hevc_frame_start(HEVCContext *s)
     if (pps->tiles_enabled_flag)
         s->local_ctx[0].end_of_tiles_x = pps->column_width[0] << sps->log2_ctb_size;
 
+    ret = export_stream_params_from_sei(s);
+    if (ret < 0)
+        return ret;
+
     ret = ff_hevc_set_new_ref(s, s->poc);
     if (ret < 0)
         goto fail;
@@ -2984,10 +2988,6 @@ static int hevc_frame_start(HEVCContext *s)
         !(s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_FILM_GRAIN) &&
         !s->avctx->hwaccel;
 
-    ret = export_stream_params_from_sei(s);
-    if (ret < 0)
-        return ret;
-
     ret = set_side_data(s);
     if (ret < 0)
         goto fail;
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/5] lavc/hevcdec: do not call export_stream_params_from_sei() in update_thread_context()

[Anton Khirnov]

It is redundant, since it only sets AVCodecContext fields that are
already copied by the generic code.
---
 libavcodec/hevc/hevcdec.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index e80f2f28c7..4a62170073 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -3637,10 +3637,6 @@ static int hevc_update_thread_context(AVCodecContext *dst,
     s->sei.common.content_light        = s0->sei.common.content_light;
     s->sei.common.aom_film_grain       = s0->sei.common.aom_film_grain;
 
-    ret = export_stream_params_from_sei(s);
-    if (ret < 0)
-        return ret;
-
     return 0;
 }
 #endif
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/5] lavc/hevcdec: do not pass a pixel format to set_sps()

[Anton Khirnov]

It is merely copied to AVCodecContext.pix_fmt, which serves no useful
purpose. set_sps() is called from two places:
* when a new SPS becomes active - then the pixel format is
  overridden immediately after the set_sps() call by the result from
  ff_get_format();
* when a new SPS is propagated across frame threads - then the
  AVCodecContext value is already set to the same value by the generic
  code.
---
 libavcodec/hevc/hevcdec.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index 4a62170073..5136bb53d9 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -526,8 +526,7 @@ static enum AVPixelFormat get_format(HEVCContext *s, const HEVCSPS *sps)
     return ff_get_format(s->avctx, pix_fmts);
 }
 
-static int set_sps(HEVCContext *s, const HEVCSPS *sps,
-                   enum AVPixelFormat pix_fmt)
+static int set_sps(HEVCContext *s, const HEVCSPS *sps)
 {
     int ret, i;
 
@@ -544,8 +543,6 @@ static int set_sps(HEVCContext *s, const HEVCSPS *sps,
 
     export_stream_params(s, sps);
 
-    s->avctx->pix_fmt = pix_fmt;
-
     ff_hevc_pred_init(&s->hpc,     sps->bit_depth);
     ff_hevc_dsp_init (&s->hevcdsp, sps->bit_depth);
     ff_videodsp_init (&s->vdsp,    sps->bit_depth);
@@ -2918,7 +2915,7 @@ static int hevc_frame_start(HEVCContext *s)
 
         ff_hevc_clear_refs(s);
 
-        ret = set_sps(s, sps, sps->pix_fmt);
+        ret = set_sps(s, sps);
         if (ret < 0)
             return ret;
 
@@ -3592,7 +3589,7 @@ static int hevc_update_thread_context(AVCodecContext *dst,
     ff_refstruct_unref(&s->pps);
 
     if (s->ps.sps != s0->ps.sps)
-        if ((ret = set_sps(s, s0->ps.sps, src->pix_fmt)) < 0)
+        if ((ret = set_sps(s, s0->ps.sps)) < 0)
             return ret;
 
     s->seq_decode = s0->seq_decode;
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/5] lavc/hevcdec: move export_stream_params() from set_sps() to hevc_frame_start()

[Anton Khirnov]

The only other caller of set_sps() --- hevc_update_thread_context() ---
does not need to call export_stream_params(), since it only updates
AVCodecContext fields that have already been updated by generic code.
---
 libavcodec/hevc/hevcdec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index 5136bb53d9..01d32086f2 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -541,8 +541,6 @@ static int set_sps(HEVCContext *s, const HEVCSPS *sps)
     if (ret < 0)
         goto fail;
 
-    export_stream_params(s, sps);
-
     ff_hevc_pred_init(&s->hpc,     sps->bit_depth);
     ff_hevc_dsp_init (&s->hevcdsp, sps->bit_depth);
     ff_videodsp_init (&s->vdsp,    sps->bit_depth);
@@ -2919,6 +2917,8 @@ static int hevc_frame_start(HEVCContext *s)
         if (ret < 0)
             return ret;
 
+        export_stream_params(s, sps);
+
         pix_fmt = get_format(s, sps);
         if (pix_fmt < 0)
             return pix_fmt;
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/5] lavc/hevcdec: improve check for PPS changing between slices

[Anton Khirnov]

Compare actual PPS objects rather than just PPS ID, as the former might
change while the latter stays the same.

Reported-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavcodec/hevc/hevcdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
index 01d32086f2..fd143cddab 100644
--- a/libavcodec/hevc/hevcdec.c
+++ b/libavcodec/hevc/hevcdec.c
@@ -603,7 +603,7 @@ static int hls_slice_header(SliceHeader *sh, const HEVCContext *s, GetBitContext
         av_log(s->avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id);
         return AVERROR_INVALIDDATA;
     }
-    if (!sh->first_slice_in_pic_flag && pps_id != sh->pps_id) {
+    if (!sh->first_slice_in_pic_flag && s->ps.pps_list[pps_id] != s->pps) {
         av_log(s->avctx, AV_LOG_ERROR, "PPS changed between slices.\n");
         return AVERROR_INVALIDDATA;
     }
-- 
2.43.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-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 5/5] lavc/hevcdec: improve check for PPS changing between slices

[Frank Plowman]

On 26/06/2024 13:43, Anton Khirnov wrote:
> Compare actual PPS objects rather than just PPS ID, as the former might
> change while the latter stays the same.
> 
> Reported-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavcodec/hevc/hevcdec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
> index 01d32086f2..fd143cddab 100644
> --- a/libavcodec/hevc/hevcdec.c
> +++ b/libavcodec/hevc/hevcdec.c
> @@ -603,7 +603,7 @@ static int hls_slice_header(SliceHeader *sh, const HEVCContext *s, GetBitContext
>          av_log(s->avctx, AV_LOG_ERROR, "PPS id out of range: %d\n", pps_id);
>          return AVERROR_INVALIDDATA;
>      }
> -    if (!sh->first_slice_in_pic_flag && pps_id != sh->pps_id) {
> +    if (!sh->first_slice_in_pic_flag && s->ps.pps_list[pps_id] != s->pps) {
>          av_log(s->avctx, AV_LOG_ERROR, "PPS changed between slices.\n");
>          return AVERROR_INVALIDDATA;
>      }

LGTM.

-- 
Frank
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/5] lavc/hevcdec: do not call export_stream_params_from_sei() in update_thread_context()

[Andreas Rheinhardt]

Anton Khirnov:
> It is redundant, since it only sets AVCodecContext fields that are
> already copied by the generic code.
> ---
>  libavcodec/hevc/hevcdec.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
> index e80f2f28c7..4a62170073 100644
> --- a/libavcodec/hevc/hevcdec.c
> +++ b/libavcodec/hevc/hevcdec.c
> @@ -3637,10 +3637,6 @@ static int hevc_update_thread_context(AVCodecContext *dst,
>      s->sei.common.content_light        = s0->sei.common.content_light;
>      s->sei.common.aom_film_grain       = s0->sei.common.aom_film_grain;
>  
> -    ret = export_stream_params_from_sei(s);
> -    if (ret < 0)
> -        return ret;
> -
>      return 0;
>  }
>  #endif

Won't this simply reopen ticket #8610 (when only patches 1+2 are applied)?

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/5] lavc/hevcdec: do not call export_stream_params_from_sei() in update_thread_context()

[Anton Khirnov]

Quoting Andreas Rheinhardt (2024-06-26 18:13:44)
> Anton Khirnov:
> > It is redundant, since it only sets AVCodecContext fields that are
> > already copied by the generic code.
> > ---
> >  libavcodec/hevc/hevcdec.c | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/libavcodec/hevc/hevcdec.c b/libavcodec/hevc/hevcdec.c
> > index e80f2f28c7..4a62170073 100644
> > --- a/libavcodec/hevc/hevcdec.c
> > +++ b/libavcodec/hevc/hevcdec.c
> > @@ -3637,10 +3637,6 @@ static int hevc_update_thread_context(AVCodecContext *dst,
> >      s->sei.common.content_light        = s0->sei.common.content_light;
> >      s->sei.common.aom_film_grain       = s0->sei.common.aom_film_grain;
> >  
> > -    ret = export_stream_params_from_sei(s);
> > -    if (ret < 0)
> > -        return ret;
> > -
> >      return 0;
> >  }
> >  #endif
> 
> Won't this simply reopen ticket #8610 (when only patches 1+2 are applied)?

I did test the sample from that issue and it exports the correct value
with any thread count. TBH I don't quite understand why the call was
added here in the first place, as it never should have been needed.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".