Re: [FFmpeg-devel] [PATCH 12/14] h264_ps: make the PPS hold a reference to its SPS

2020-04-06 Thread Michael Niedermayer
On Fri, Mar 27, 2020 at 01:57:46PM +0100, Anton Khirnov wrote:
> It represents the relationship between them more naturally and will be
> useful in the following commits.
> 
> Allows significantly more frames in fate-h264-attachment-631 to be
> decoded.
> ---
>  libavcodec/h264_parser.c   |  16 +---
>  libavcodec/h264_ps.c   |  30 +-
>  libavcodec/h264_ps.h   |   4 +-
>  libavcodec/h264_slice.c|  27 +-
>  libavcodec/h264dec.c   |   4 +-
>  tests/ref/fate/h264-attachment-631 | 148 +
>  6 files changed, 184 insertions(+), 45 deletions(-)

Assuming this breaks no h264 files, LGTM

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr


signature.asc
Description: PGP signature
___
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 12/14] h264_ps: make the PPS hold a reference to its SPS

2020-03-27 Thread Anton Khirnov
It represents the relationship between them more naturally and will be
useful in the following commits.

Allows significantly more frames in fate-h264-attachment-631 to be
decoded.
---
 libavcodec/h264_parser.c   |  16 +---
 libavcodec/h264_ps.c   |  30 +-
 libavcodec/h264_ps.h   |   4 +-
 libavcodec/h264_slice.c|  27 +-
 libavcodec/h264dec.c   |   4 +-
 tests/ref/fate/h264-attachment-631 | 148 +
 6 files changed, 184 insertions(+), 45 deletions(-)

diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index ec1cbc6a66..aacd44cf3b 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -361,26 +361,14 @@ static inline int parse_nal_units(AVCodecParserContext *s,
 }
 
 av_buffer_unref(>ps.pps_ref);
-av_buffer_unref(>ps.sps_ref);
 p->ps.pps = NULL;
 p->ps.sps = NULL;
 p->ps.pps_ref = av_buffer_ref(p->ps.pps_list[pps_id]);
 if (!p->ps.pps_ref)
 goto fail;
 p->ps.pps = (const PPS*)p->ps.pps_ref->data;
-
-if (!p->ps.sps_list[p->ps.pps->sps_id]) {
-av_log(avctx, AV_LOG_ERROR,
-   "non-existing SPS %u referenced\n", p->ps.pps->sps_id);
-goto fail;
-}
-
-p->ps.sps_ref = av_buffer_ref(p->ps.sps_list[p->ps.pps->sps_id]);
-if (!p->ps.sps_ref)
-goto fail;
-p->ps.sps = (const SPS*)p->ps.sps_ref->data;
-
-sps = p->ps.sps;
+p->ps.sps = p->ps.pps->sps;
+sps   = p->ps.sps;
 
 // heuristic to detect non marked keyframes
 if (p->ps.sps->ref_frame_count <= 1 && p->ps.pps->ref_count[0] <= 
1 && s->pict_type == AV_PICTURE_TYPE_I)
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index 8df195e0a9..e774929e21 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -324,7 +324,6 @@ void ff_h264_ps_uninit(H264ParamSets *ps)
 for (i = 0; i < MAX_PPS_COUNT; i++)
 av_buffer_unref(>pps_list[i]);
 
-av_buffer_unref(>sps_ref);
 av_buffer_unref(>pps_ref);
 
 ps->pps = NULL;
@@ -738,6 +737,15 @@ static int more_rbsp_data_in_pps(const SPS *sps, void 
*logctx)
 return 1;
 }
 
+static void pps_free(void *opaque, uint8_t *data)
+{
+PPS *pps = (PPS*)data;
+
+av_buffer_unref(>sps_ref);
+
+av_freep();
+}
+
 int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext 
*avctx,
  H264ParamSets *ps, int bit_length)
 {
@@ -754,10 +762,15 @@ int ff_h264_decode_picture_parameter_set(GetBitContext 
*gb, AVCodecContext *avct
 return AVERROR_INVALIDDATA;
 }
 
-pps_buf = av_buffer_allocz(sizeof(*pps));
-if (!pps_buf)
+pps = av_mallocz(sizeof(*pps));
+if (!pps)
 return AVERROR(ENOMEM);
-pps = (PPS*)pps_buf->data;
+pps_buf = av_buffer_create((uint8_t*)pps, sizeof(*pps),
+   pps_free, NULL, 0);
+if (!pps_buf) {
+av_freep();
+return AVERROR(ENOMEM);
+}
 
 pps->data_size = gb->buffer_end - gb->buffer;
 if (pps->data_size > sizeof(pps->data)) {
@@ -775,7 +788,14 @@ int ff_h264_decode_picture_parameter_set(GetBitContext 
*gb, AVCodecContext *avct
 ret = AVERROR_INVALIDDATA;
 goto fail;
 }
-sps = (const SPS*)ps->sps_list[pps->sps_id]->data;
+pps->sps_ref = av_buffer_ref(ps->sps_list[pps->sps_id]);
+if (!pps->sps_ref) {
+ret = AVERROR(ENOMEM);
+goto fail;
+}
+pps->sps = (const SPS*)pps->sps_ref->data;
+sps  = pps->sps;
+
 if (sps->bit_depth_luma > 14) {
 av_log(avctx, AV_LOG_ERROR,
"Invalid luma bit depth=%d\n",
diff --git a/libavcodec/h264_ps.h b/libavcodec/h264_ps.h
index d6798ca0ef..3f1ab72e38 100644
--- a/libavcodec/h264_ps.h
+++ b/libavcodec/h264_ps.h
@@ -135,6 +135,9 @@ typedef struct PPS {
 uint32_t dequant8_buffer[6][QP_MAX_NUM + 1][64];
 uint32_t(*dequant4_coeff[6])[16];
 uint32_t(*dequant8_coeff[6])[64];
+
+AVBufferRef *sps_ref;
+const SPS   *sps;
 } PPS;
 
 typedef struct H264ParamSets {
@@ -142,7 +145,6 @@ typedef struct H264ParamSets {
 AVBufferRef *pps_list[MAX_PPS_COUNT];
 
 AVBufferRef *pps_ref;
-AVBufferRef *sps_ref;
 /* currently active parameters sets */
 const PPS *pps;
 const SPS *sps;
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index c6072738d7..5a8a4a7f86 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -333,7 +333,6 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
 }
 
 av_buffer_unref(>ps.pps_ref);
-av_buffer_unref(>ps.sps_ref);
 h->ps.pps = NULL;
 h->ps.sps = NULL;
 if (h1->ps.pps_ref) {
@@ -341,12 +340,7 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
 if (!h->ps.pps_ref)
 return