| Message ID | 20180508204856.10796-2-jamrial@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On 08/05/18 21:48, James Almer wrote: > This is more in line to how h264_ps handles it. > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Not really going to make much of a difference seeing parameter sets > rarely show up in-band even in raw streams, but it's one less memcpy > anyway. > > libavcodec/cbs_h264.h | 4 +-- > libavcodec/cbs_h2645.c | 41 ++++++++++++++------------- > libavcodec/cbs_h264_syntax_template.c | 19 ++++++------- > libavcodec/cbs_h265.h | 6 ++-- > libavcodec/cbs_h265_syntax_template.c | 21 +++++++------- > 5 files changed, 44 insertions(+), 47 deletions(-) > > diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h > index becea3adfe..239f3b6b90 100644 > --- a/libavcodec/cbs_h264.h > +++ b/libavcodec/cbs_h264.h > @@ -420,8 +420,8 @@ typedef struct CodedBitstreamH264Context { > > // All currently available parameter sets. These are updated when > // any parameter set NAL unit is read/written with this context. > - H264RawSPS *sps[H264_MAX_SPS_COUNT]; > - H264RawPPS *pps[H264_MAX_PPS_COUNT]; > + AVBufferRef *sps[H264_MAX_SPS_COUNT]; > + AVBufferRef *pps[H264_MAX_PPS_COUNT]; Can we keep the current xps variables as they are and add a separate xps_ref? That doesn't significantly increase the complexity of the replacement functions, and would avoid the ugly casting in the read/write templated code (indeed, I think it removes all changes there entirely). > > // The currently active parameter sets. These are updated when any > // NAL unit refers to the relevant parameter set. These pointers > diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c > index 580ca09f63..6c81a2e1b3 100644 > --- a/libavcodec/cbs_h2645.c > +++ b/libavcodec/cbs_h2645.c > @@ -676,22 +676,23 @@ static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx, > > #define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element) \ > static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \ > - const H26 ## h26n ## Raw ## ps_name *ps_var) \ > + CodedBitstreamUnit *unit) \ > { \ > CodedBitstreamH26 ## h26n ## Context *priv = ctx->priv_data; \ > + H26 ## h26n ## Raw ## ps_name *ps_var = unit->content; \ > unsigned int id = ps_var->id_element; \ > if (id > FF_ARRAY_ELEMS(priv->ps_var)) { \ > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid " #ps_name \ > " id : %d.\n", id); \ > return AVERROR_INVALIDDATA; \ > } \ > - if (priv->ps_var[id] == priv->active_ ## ps_var) \ > + if (priv->ps_var[id] && \ > + (H26 ## h26n ## Raw ## ps_name *) priv->ps_var[id]->data == priv->active_ ## ps_var) \ > priv->active_ ## ps_var = NULL ; \ > - av_freep(&priv->ps_var[id]); \ > - priv->ps_var[id] = av_malloc(sizeof(*ps_var)); \ > + av_buffer_unref(&priv->ps_var[id]); \ > + priv->ps_var[id] = av_buffer_ref(unit->content_ref); \ > if (!priv->ps_var[id]) \ > return AVERROR(ENOMEM); \ > - memcpy(priv->ps_var[id], ps_var, sizeof(*ps_var)); \ > return 0; \ > } > > @@ -725,7 +726,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h264_replace_sps(ctx, sps); > + err = cbs_h264_replace_sps(ctx, unit); > if (err < 0) > return err; > } > @@ -759,7 +760,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h264_replace_pps(ctx, pps); > + err = cbs_h264_replace_pps(ctx, unit); > if (err < 0) > return err; > } > @@ -872,7 +873,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h265_replace_vps(ctx, vps); > + err = cbs_h265_replace_vps(ctx, unit); > if (err < 0) > return err; > } > @@ -891,7 +892,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h265_replace_sps(ctx, sps); > + err = cbs_h265_replace_sps(ctx, unit); > if (err < 0) > return err; > } > @@ -911,7 +912,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h265_replace_pps(ctx, pps); > + err = cbs_h265_replace_pps(ctx, unit); > if (err < 0) > return err; > } > @@ -1001,7 +1002,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h264_replace_sps(ctx, sps); > + err = cbs_h264_replace_sps(ctx, unit); > if (err < 0) > return err; > } > @@ -1025,7 +1026,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h264_replace_pps(ctx, pps); > + err = cbs_h264_replace_pps(ctx, unit); > if (err < 0) > return err; > } > @@ -1122,7 +1123,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h265_replace_vps(ctx, vps); > + err = cbs_h265_replace_vps(ctx, unit); > if (err < 0) > return err; > } > @@ -1136,7 +1137,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h265_replace_sps(ctx, sps); > + err = cbs_h265_replace_sps(ctx, unit); > if (err < 0) > return err; > } > @@ -1150,7 +1151,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, > if (err < 0) > return err; > > - err = cbs_h265_replace_pps(ctx, pps); > + err = cbs_h265_replace_pps(ctx, unit); > if (err < 0) > return err; > } > @@ -1384,9 +1385,9 @@ static void cbs_h264_close(CodedBitstreamContext *ctx) > av_freep(&h264->common.write_buffer); > > for (i = 0; i < FF_ARRAY_ELEMS(h264->sps); i++) > - av_freep(&h264->sps[i]); > + av_buffer_unref(&h264->sps[i]); > for (i = 0; i < FF_ARRAY_ELEMS(h264->pps); i++) > - av_freep(&h264->pps[i]); > + av_buffer_unref(&h264->pps[i]); > } > > static void cbs_h265_close(CodedBitstreamContext *ctx) > @@ -1399,11 +1400,11 @@ static void cbs_h265_close(CodedBitstreamContext *ctx) > av_freep(&h265->common.write_buffer); > > for (i = 0; i < FF_ARRAY_ELEMS(h265->vps); i++) > - av_freep(&h265->vps[i]); > + av_buffer_unref(&h265->vps[i]); > for (i = 0; i < FF_ARRAY_ELEMS(h265->sps); i++) > - av_freep(&h265->sps[i]); > + av_buffer_unref(&h265->sps[i]); > for (i = 0; i < FF_ARRAY_ELEMS(h265->pps); i++) > - av_freep(&h265->pps[i]); > + av_buffer_unref(&h265->pps[i]); > } > > const CodedBitstreamType ff_cbs_type_h264 = { > diff --git a/libavcodec/cbs_h264_syntax_template.c b/libavcodec/cbs_h264_syntax_template.c > index c3327f6e90..6e4155baac 100644 > --- a/libavcodec/cbs_h264_syntax_template.c > +++ b/libavcodec/cbs_h264_syntax_template.c > @@ -368,7 +368,7 @@ static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext *rw, > ue(pic_parameter_set_id, 0, 255); > ue(seq_parameter_set_id, 0, 31); > > - sps = h264->sps[current->seq_parameter_set_id]; > + sps = (const H264RawSPS *)h264->sps[current->seq_parameter_set_id]->data; > if (!sps) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", > current->seq_parameter_set_id); > @@ -471,13 +471,12 @@ static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw, > > ue(seq_parameter_set_id, 0, 31); > > - sps = h264->sps[current->seq_parameter_set_id]; > - if (!sps) { > + if (!h264->sps[current->seq_parameter_set_id]) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", > current->seq_parameter_set_id); > return AVERROR_INVALIDDATA; > } > - h264->active_sps = sps; > + h264->active_sps = sps = (const H264RawSPS *)h264->sps[current->seq_parameter_set_id]->data; > > if (sps->vui.nal_hrd_parameters_present_flag) { > for (i = 0; i <= sps->vui.nal_hrd_parameters.cpb_cnt_minus1; i++) { > @@ -578,7 +577,7 @@ static int FUNC(sei_pic_timing)(CodedBitstreamContext *ctx, RWContext *rw, > } > } > if (k >= 0) > - sps = h264->sps[k]; > + sps = (const H264RawSPS *)h264->sps[k]->data; > } > if (!sps) { > av_log(ctx->log_ctx, AV_LOG_ERROR, > @@ -1100,21 +1099,19 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, > > ue(pic_parameter_set_id, 0, 255); > > - pps = h264->pps[current->pic_parameter_set_id]; > - if (!pps) { > + if (!h264->pps[current->pic_parameter_set_id]) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "PPS id %d not available.\n", > current->pic_parameter_set_id); > return AVERROR_INVALIDDATA; > } > - h264->active_pps = pps; > + h264->active_pps = pps = (const H264RawPPS *)h264->pps[current->pic_parameter_set_id]->data; > > - sps = h264->sps[pps->seq_parameter_set_id]; > - if (!sps) { > + if (!h264->sps[pps->seq_parameter_set_id]) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", > pps->seq_parameter_set_id); > return AVERROR_INVALIDDATA; > } > - h264->active_sps = sps; > + h264->active_sps = sps = (const H264RawSPS *)h264->sps[pps->seq_parameter_set_id]->data; > > if (sps->separate_colour_plane_flag) > u(2, colour_plane_id, 0, 2); > diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h > index 1b357293ab..4eec99e9a8 100644 > --- a/libavcodec/cbs_h265.h > +++ b/libavcodec/cbs_h265.h > @@ -522,9 +522,9 @@ typedef struct CodedBitstreamH265Context { > > // All currently available parameter sets. These are updated when > // any parameter set NAL unit is read/written with this context. > - H265RawVPS *vps[HEVC_MAX_VPS_COUNT]; > - H265RawSPS *sps[HEVC_MAX_SPS_COUNT]; > - H265RawPPS *pps[HEVC_MAX_PPS_COUNT]; > + AVBufferRef *vps[HEVC_MAX_VPS_COUNT]; > + AVBufferRef *sps[HEVC_MAX_SPS_COUNT]; > + AVBufferRef *pps[HEVC_MAX_PPS_COUNT]; > > // The currently active parameter sets. These are updated when any > // NAL unit refers to the relevant parameter set. These pointers > diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c > index 9f13061f38..bc9e6ddf08 100644 > --- a/libavcodec/cbs_h265_syntax_template.c > +++ b/libavcodec/cbs_h265_syntax_template.c > @@ -690,11 +690,12 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, > CHECK(FUNC(nal_unit_header)(ctx, rw, ¤t->nal_unit_header, HEVC_NAL_SPS)); > > u(4, sps_video_parameter_set_id, 0, 15); > - h265->active_vps = vps = h265->vps[current->sps_video_parameter_set_id]; > + h265->active_vps = vps = NULL; > > u(3, sps_max_sub_layers_minus1, 0, HEVC_MAX_SUB_LAYERS - 1); > flag(sps_temporal_id_nesting_flag); > - if (vps) { > + if (h265->vps[current->sps_video_parameter_set_id]) { > + h265->active_vps = vps = (const H265RawVPS *)h265->vps[current->sps_video_parameter_set_id]->data; > if (vps->vps_max_sub_layers_minus1 > current->sps_max_sub_layers_minus1) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid stream: " > "sps_max_sub_layers_minus1 (%d) must be less than or equal to " > @@ -949,13 +950,13 @@ static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext *rw, > > ue(pps_pic_parameter_set_id, 0, 63); > ue(pps_seq_parameter_set_id, 0, 15); > - sps = h265->sps[current->pps_seq_parameter_set_id]; > - if (!sps) { > + > + if (!h265->sps[current->pps_seq_parameter_set_id]) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", > current->pps_seq_parameter_set_id); > return AVERROR_INVALIDDATA; > } > - h265->active_sps = sps; > + h265->active_sps = sps = (const H265RawSPS *)h265->sps[current->pps_seq_parameter_set_id]->data; > > flag(dependent_slice_segments_enabled_flag); > flag(output_flag_present_flag); > @@ -1224,21 +1225,19 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, > > ue(slice_pic_parameter_set_id, 0, 63); > > - pps = h265->pps[current->slice_pic_parameter_set_id]; > - if (!pps) { > + if (!h265->pps[current->slice_pic_parameter_set_id]) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "PPS id %d not available.\n", > current->slice_pic_parameter_set_id); > return AVERROR_INVALIDDATA; > } > - h265->active_pps = pps; > + h265->active_pps = pps = (const H265RawPPS *)h265->pps[current->slice_pic_parameter_set_id]->data; > > - sps = h265->sps[pps->pps_seq_parameter_set_id]; > - if (!sps) { > + if (!h265->sps[pps->pps_seq_parameter_set_id]) { > av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", > pps->pps_seq_parameter_set_id); > return AVERROR_INVALIDDATA; > } > - h265->active_sps = sps; > + h265->active_sps = sps = (const H265RawSPS *)h265->sps[pps->pps_seq_parameter_set_id]->data; > > min_cb_log2_size_y = sps->log2_min_luma_coding_block_size_minus3 + 3; > ctb_log2_size_y = min_cb_log2_size_y + sps->log2_diff_max_min_luma_coding_block_size; > Good change. (I'm realising now that it fixes the possible dangling references held in pointers in the stored parameter sets too, which I hadn't thought about before.) Thanks, - Mark
On 5/8/2018 7:37 PM, Mark Thompson wrote: > On 08/05/18 21:48, James Almer wrote: >> This is more in line to how h264_ps handles it. >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> Not really going to make much of a difference seeing parameter sets >> rarely show up in-band even in raw streams, but it's one less memcpy >> anyway. >> >> libavcodec/cbs_h264.h | 4 +-- >> libavcodec/cbs_h2645.c | 41 ++++++++++++++------------- >> libavcodec/cbs_h264_syntax_template.c | 19 ++++++------- >> libavcodec/cbs_h265.h | 6 ++-- >> libavcodec/cbs_h265_syntax_template.c | 21 +++++++------- >> 5 files changed, 44 insertions(+), 47 deletions(-) >> >> diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h >> index becea3adfe..239f3b6b90 100644 >> --- a/libavcodec/cbs_h264.h >> +++ b/libavcodec/cbs_h264.h >> @@ -420,8 +420,8 @@ typedef struct CodedBitstreamH264Context { >> >> // All currently available parameter sets. These are updated when >> // any parameter set NAL unit is read/written with this context. >> - H264RawSPS *sps[H264_MAX_SPS_COUNT]; >> - H264RawPPS *pps[H264_MAX_PPS_COUNT]; >> + AVBufferRef *sps[H264_MAX_SPS_COUNT]; >> + AVBufferRef *pps[H264_MAX_PPS_COUNT]; > > Can we keep the current xps variables as they are and add a separate xps_ref? That doesn't significantly increase the complexity of the replacement functions, and would avoid the ugly casting in the read/write templated code (indeed, I think it removes all changes there entirely). If you mean making it AVBufferRef *sps_ref[H264_MAX_SPS_COUNT]; AVBufferRef *pps_ref[H264_MAX_PPS_COUNT]; H264RawSPS *sps[H264_MAX_SPS_COUNT]; H264RawPPS *pps[H264_MAX_PPS_COUNT]; Then sure, it indeed simplifies the patch a lot. Will send an updated version in a moment. > >> >> // The currently active parameter sets. These are updated when any >> // NAL unit refers to the relevant parameter set. These pointers >> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c >> index 580ca09f63..6c81a2e1b3 100644 >> --- a/libavcodec/cbs_h2645.c >> +++ b/libavcodec/cbs_h2645.c >> @@ -676,22 +676,23 @@ static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx, >> >> #define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element) \ >> static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \ >> - const H26 ## h26n ## Raw ## ps_name *ps_var) \ >> + CodedBitstreamUnit *unit) \ >> { \ >> CodedBitstreamH26 ## h26n ## Context *priv = ctx->priv_data; \ >> + H26 ## h26n ## Raw ## ps_name *ps_var = unit->content; \ >> unsigned int id = ps_var->id_element; \ >> if (id > FF_ARRAY_ELEMS(priv->ps_var)) { \ >> av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid " #ps_name \ >> " id : %d.\n", id); \ >> return AVERROR_INVALIDDATA; \ >> } \ >> - if (priv->ps_var[id] == priv->active_ ## ps_var) \ >> + if (priv->ps_var[id] && \ >> + (H26 ## h26n ## Raw ## ps_name *) priv->ps_var[id]->data == priv->active_ ## ps_var) \ >> priv->active_ ## ps_var = NULL ; \ >> - av_freep(&priv->ps_var[id]); \ >> - priv->ps_var[id] = av_malloc(sizeof(*ps_var)); \ >> + av_buffer_unref(&priv->ps_var[id]); \ >> + priv->ps_var[id] = av_buffer_ref(unit->content_ref); \ >> if (!priv->ps_var[id]) \ >> return AVERROR(ENOMEM); \ >> - memcpy(priv->ps_var[id], ps_var, sizeof(*ps_var)); \ >> return 0; \ >> } >> >> @@ -725,7 +726,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h264_replace_sps(ctx, sps); >> + err = cbs_h264_replace_sps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -759,7 +760,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h264_replace_pps(ctx, pps); >> + err = cbs_h264_replace_pps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -872,7 +873,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h265_replace_vps(ctx, vps); >> + err = cbs_h265_replace_vps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -891,7 +892,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h265_replace_sps(ctx, sps); >> + err = cbs_h265_replace_sps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -911,7 +912,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h265_replace_pps(ctx, pps); >> + err = cbs_h265_replace_pps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -1001,7 +1002,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h264_replace_sps(ctx, sps); >> + err = cbs_h264_replace_sps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -1025,7 +1026,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h264_replace_pps(ctx, pps); >> + err = cbs_h264_replace_pps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -1122,7 +1123,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h265_replace_vps(ctx, vps); >> + err = cbs_h265_replace_vps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -1136,7 +1137,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h265_replace_sps(ctx, sps); >> + err = cbs_h265_replace_sps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -1150,7 +1151,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, >> if (err < 0) >> return err; >> >> - err = cbs_h265_replace_pps(ctx, pps); >> + err = cbs_h265_replace_pps(ctx, unit); >> if (err < 0) >> return err; >> } >> @@ -1384,9 +1385,9 @@ static void cbs_h264_close(CodedBitstreamContext *ctx) >> av_freep(&h264->common.write_buffer); >> >> for (i = 0; i < FF_ARRAY_ELEMS(h264->sps); i++) >> - av_freep(&h264->sps[i]); >> + av_buffer_unref(&h264->sps[i]); >> for (i = 0; i < FF_ARRAY_ELEMS(h264->pps); i++) >> - av_freep(&h264->pps[i]); >> + av_buffer_unref(&h264->pps[i]); >> } >> >> static void cbs_h265_close(CodedBitstreamContext *ctx) >> @@ -1399,11 +1400,11 @@ static void cbs_h265_close(CodedBitstreamContext *ctx) >> av_freep(&h265->common.write_buffer); >> >> for (i = 0; i < FF_ARRAY_ELEMS(h265->vps); i++) >> - av_freep(&h265->vps[i]); >> + av_buffer_unref(&h265->vps[i]); >> for (i = 0; i < FF_ARRAY_ELEMS(h265->sps); i++) >> - av_freep(&h265->sps[i]); >> + av_buffer_unref(&h265->sps[i]); >> for (i = 0; i < FF_ARRAY_ELEMS(h265->pps); i++) >> - av_freep(&h265->pps[i]); >> + av_buffer_unref(&h265->pps[i]); >> } >> >> const CodedBitstreamType ff_cbs_type_h264 = { >> diff --git a/libavcodec/cbs_h264_syntax_template.c b/libavcodec/cbs_h264_syntax_template.c >> index c3327f6e90..6e4155baac 100644 >> --- a/libavcodec/cbs_h264_syntax_template.c >> +++ b/libavcodec/cbs_h264_syntax_template.c >> @@ -368,7 +368,7 @@ static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext *rw, >> ue(pic_parameter_set_id, 0, 255); >> ue(seq_parameter_set_id, 0, 31); >> >> - sps = h264->sps[current->seq_parameter_set_id]; >> + sps = (const H264RawSPS *)h264->sps[current->seq_parameter_set_id]->data; >> if (!sps) { >> av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", >> current->seq_parameter_set_id); >> @@ -471,13 +471,12 @@ static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw, >> >> ue(seq_parameter_set_id, 0, 31); >> >> - sps = h264->sps[current->seq_parameter_set_id]; >> - if (!sps) { >> + if (!h264->sps[current->seq_parameter_set_id]) { >> av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", >> current->seq_parameter_set_id); >> return AVERROR_INVALIDDATA; >> } >> - h264->active_sps = sps; >> + h264->active_sps = sps = (const H264RawSPS *)h264->sps[current->seq_parameter_set_id]->data; >> >> if (sps->vui.nal_hrd_parameters_present_flag) { >> for (i = 0; i <= sps->vui.nal_hrd_parameters.cpb_cnt_minus1; i++) { >> @@ -578,7 +577,7 @@ static int FUNC(sei_pic_timing)(CodedBitstreamContext *ctx, RWContext *rw, >> } >> } >> if (k >= 0) >> - sps = h264->sps[k]; >> + sps = (const H264RawSPS *)h264->sps[k]->data; >> } >> if (!sps) { >> av_log(ctx->log_ctx, AV_LOG_ERROR, >> @@ -1100,21 +1099,19 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, >> >> ue(pic_parameter_set_id, 0, 255); >> >> - pps = h264->pps[current->pic_parameter_set_id]; >> - if (!pps) { >> + if (!h264->pps[current->pic_parameter_set_id]) { >> av_log(ctx->log_ctx, AV_LOG_ERROR, "PPS id %d not available.\n", >> current->pic_parameter_set_id); >> return AVERROR_INVALIDDATA; >> } >> - h264->active_pps = pps; >> + h264->active_pps = pps = (const H264RawPPS *)h264->pps[current->pic_parameter_set_id]->data; >> >> - sps = h264->sps[pps->seq_parameter_set_id]; >> - if (!sps) { >> + if (!h264->sps[pps->seq_parameter_set_id]) { >> av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", >> pps->seq_parameter_set_id); >> return AVERROR_INVALIDDATA; >> } >> - h264->active_sps = sps; >> + h264->active_sps = sps = (const H264RawSPS *)h264->sps[pps->seq_parameter_set_id]->data; >> >> if (sps->separate_colour_plane_flag) >> u(2, colour_plane_id, 0, 2); >> diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h >> index 1b357293ab..4eec99e9a8 100644 >> --- a/libavcodec/cbs_h265.h >> +++ b/libavcodec/cbs_h265.h >> @@ -522,9 +522,9 @@ typedef struct CodedBitstreamH265Context { >> >> // All currently available parameter sets. These are updated when >> // any parameter set NAL unit is read/written with this context. >> - H265RawVPS *vps[HEVC_MAX_VPS_COUNT]; >> - H265RawSPS *sps[HEVC_MAX_SPS_COUNT]; >> - H265RawPPS *pps[HEVC_MAX_PPS_COUNT]; >> + AVBufferRef *vps[HEVC_MAX_VPS_COUNT]; >> + AVBufferRef *sps[HEVC_MAX_SPS_COUNT]; >> + AVBufferRef *pps[HEVC_MAX_PPS_COUNT]; >> >> // The currently active parameter sets. These are updated when any >> // NAL unit refers to the relevant parameter set. These pointers >> diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c >> index 9f13061f38..bc9e6ddf08 100644 >> --- a/libavcodec/cbs_h265_syntax_template.c >> +++ b/libavcodec/cbs_h265_syntax_template.c >> @@ -690,11 +690,12 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, >> CHECK(FUNC(nal_unit_header)(ctx, rw, ¤t->nal_unit_header, HEVC_NAL_SPS)); >> >> u(4, sps_video_parameter_set_id, 0, 15); >> - h265->active_vps = vps = h265->vps[current->sps_video_parameter_set_id]; >> + h265->active_vps = vps = NULL; >> >> u(3, sps_max_sub_layers_minus1, 0, HEVC_MAX_SUB_LAYERS - 1); >> flag(sps_temporal_id_nesting_flag); >> - if (vps) { >> + if (h265->vps[current->sps_video_parameter_set_id]) { >> + h265->active_vps = vps = (const H265RawVPS *)h265->vps[current->sps_video_parameter_set_id]->data; >> if (vps->vps_max_sub_layers_minus1 > current->sps_max_sub_layers_minus1) { >> av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid stream: " >> "sps_max_sub_layers_minus1 (%d) must be less than or equal to " >> @@ -949,13 +950,13 @@ static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext *rw, >> >> ue(pps_pic_parameter_set_id, 0, 63); >> ue(pps_seq_parameter_set_id, 0, 15); >> - sps = h265->sps[current->pps_seq_parameter_set_id]; >> - if (!sps) { >> + >> + if (!h265->sps[current->pps_seq_parameter_set_id]) { >> av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", >> current->pps_seq_parameter_set_id); >> return AVERROR_INVALIDDATA; >> } >> - h265->active_sps = sps; >> + h265->active_sps = sps = (const H265RawSPS *)h265->sps[current->pps_seq_parameter_set_id]->data; >> >> flag(dependent_slice_segments_enabled_flag); >> flag(output_flag_present_flag); >> @@ -1224,21 +1225,19 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, >> >> ue(slice_pic_parameter_set_id, 0, 63); >> >> - pps = h265->pps[current->slice_pic_parameter_set_id]; >> - if (!pps) { >> + if (!h265->pps[current->slice_pic_parameter_set_id]) { >> av_log(ctx->log_ctx, AV_LOG_ERROR, "PPS id %d not available.\n", >> current->slice_pic_parameter_set_id); >> return AVERROR_INVALIDDATA; >> } >> - h265->active_pps = pps; >> + h265->active_pps = pps = (const H265RawPPS *)h265->pps[current->slice_pic_parameter_set_id]->data; >> >> - sps = h265->sps[pps->pps_seq_parameter_set_id]; >> - if (!sps) { >> + if (!h265->sps[pps->pps_seq_parameter_set_id]) { >> av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", >> pps->pps_seq_parameter_set_id); >> return AVERROR_INVALIDDATA; >> } >> - h265->active_sps = sps; >> + h265->active_sps = sps = (const H265RawSPS *)h265->sps[pps->pps_seq_parameter_set_id]->data; >> >> min_cb_log2_size_y = sps->log2_min_luma_coding_block_size_minus3 + 3; >> ctb_log2_size_y = min_cb_log2_size_y + sps->log2_diff_max_min_luma_coding_block_size; >> > > Good change. (I'm realising now that it fixes the possible dangling references held in pointers in the stored parameter sets too, which I hadn't thought about before.) > > Thanks, > > - Mark > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h index becea3adfe..239f3b6b90 100644 --- a/libavcodec/cbs_h264.h +++ b/libavcodec/cbs_h264.h @@ -420,8 +420,8 @@ typedef struct CodedBitstreamH264Context { // All currently available parameter sets. These are updated when // any parameter set NAL unit is read/written with this context. - H264RawSPS *sps[H264_MAX_SPS_COUNT]; - H264RawPPS *pps[H264_MAX_PPS_COUNT]; + AVBufferRef *sps[H264_MAX_SPS_COUNT]; + AVBufferRef *pps[H264_MAX_PPS_COUNT]; // The currently active parameter sets. These are updated when any // NAL unit refers to the relevant parameter set. These pointers diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c index 580ca09f63..6c81a2e1b3 100644 --- a/libavcodec/cbs_h2645.c +++ b/libavcodec/cbs_h2645.c @@ -676,22 +676,23 @@ static int cbs_h2645_split_fragment(CodedBitstreamContext *ctx, #define cbs_h2645_replace_ps(h26n, ps_name, ps_var, id_element) \ static int cbs_h26 ## h26n ## _replace_ ## ps_var(CodedBitstreamContext *ctx, \ - const H26 ## h26n ## Raw ## ps_name *ps_var) \ + CodedBitstreamUnit *unit) \ { \ CodedBitstreamH26 ## h26n ## Context *priv = ctx->priv_data; \ + H26 ## h26n ## Raw ## ps_name *ps_var = unit->content; \ unsigned int id = ps_var->id_element; \ if (id > FF_ARRAY_ELEMS(priv->ps_var)) { \ av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid " #ps_name \ " id : %d.\n", id); \ return AVERROR_INVALIDDATA; \ } \ - if (priv->ps_var[id] == priv->active_ ## ps_var) \ + if (priv->ps_var[id] && \ + (H26 ## h26n ## Raw ## ps_name *) priv->ps_var[id]->data == priv->active_ ## ps_var) \ priv->active_ ## ps_var = NULL ; \ - av_freep(&priv->ps_var[id]); \ - priv->ps_var[id] = av_malloc(sizeof(*ps_var)); \ + av_buffer_unref(&priv->ps_var[id]); \ + priv->ps_var[id] = av_buffer_ref(unit->content_ref); \ if (!priv->ps_var[id]) \ return AVERROR(ENOMEM); \ - memcpy(priv->ps_var[id], ps_var, sizeof(*ps_var)); \ return 0; \ } @@ -725,7 +726,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_sps(ctx, sps); + err = cbs_h264_replace_sps(ctx, unit); if (err < 0) return err; } @@ -759,7 +760,7 @@ static int cbs_h264_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_pps(ctx, pps); + err = cbs_h264_replace_pps(ctx, unit); if (err < 0) return err; } @@ -872,7 +873,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_vps(ctx, vps); + err = cbs_h265_replace_vps(ctx, unit); if (err < 0) return err; } @@ -891,7 +892,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_sps(ctx, sps); + err = cbs_h265_replace_sps(ctx, unit); if (err < 0) return err; } @@ -911,7 +912,7 @@ static int cbs_h265_read_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_pps(ctx, pps); + err = cbs_h265_replace_pps(ctx, unit); if (err < 0) return err; } @@ -1001,7 +1002,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_sps(ctx, sps); + err = cbs_h264_replace_sps(ctx, unit); if (err < 0) return err; } @@ -1025,7 +1026,7 @@ static int cbs_h264_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h264_replace_pps(ctx, pps); + err = cbs_h264_replace_pps(ctx, unit); if (err < 0) return err; } @@ -1122,7 +1123,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_vps(ctx, vps); + err = cbs_h265_replace_vps(ctx, unit); if (err < 0) return err; } @@ -1136,7 +1137,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_sps(ctx, sps); + err = cbs_h265_replace_sps(ctx, unit); if (err < 0) return err; } @@ -1150,7 +1151,7 @@ static int cbs_h265_write_nal_unit(CodedBitstreamContext *ctx, if (err < 0) return err; - err = cbs_h265_replace_pps(ctx, pps); + err = cbs_h265_replace_pps(ctx, unit); if (err < 0) return err; } @@ -1384,9 +1385,9 @@ static void cbs_h264_close(CodedBitstreamContext *ctx) av_freep(&h264->common.write_buffer); for (i = 0; i < FF_ARRAY_ELEMS(h264->sps); i++) - av_freep(&h264->sps[i]); + av_buffer_unref(&h264->sps[i]); for (i = 0; i < FF_ARRAY_ELEMS(h264->pps); i++) - av_freep(&h264->pps[i]); + av_buffer_unref(&h264->pps[i]); } static void cbs_h265_close(CodedBitstreamContext *ctx) @@ -1399,11 +1400,11 @@ static void cbs_h265_close(CodedBitstreamContext *ctx) av_freep(&h265->common.write_buffer); for (i = 0; i < FF_ARRAY_ELEMS(h265->vps); i++) - av_freep(&h265->vps[i]); + av_buffer_unref(&h265->vps[i]); for (i = 0; i < FF_ARRAY_ELEMS(h265->sps); i++) - av_freep(&h265->sps[i]); + av_buffer_unref(&h265->sps[i]); for (i = 0; i < FF_ARRAY_ELEMS(h265->pps); i++) - av_freep(&h265->pps[i]); + av_buffer_unref(&h265->pps[i]); } const CodedBitstreamType ff_cbs_type_h264 = { diff --git a/libavcodec/cbs_h264_syntax_template.c b/libavcodec/cbs_h264_syntax_template.c index c3327f6e90..6e4155baac 100644 --- a/libavcodec/cbs_h264_syntax_template.c +++ b/libavcodec/cbs_h264_syntax_template.c @@ -368,7 +368,7 @@ static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext *rw, ue(pic_parameter_set_id, 0, 255); ue(seq_parameter_set_id, 0, 31); - sps = h264->sps[current->seq_parameter_set_id]; + sps = (const H264RawSPS *)h264->sps[current->seq_parameter_set_id]->data; if (!sps) { av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", current->seq_parameter_set_id); @@ -471,13 +471,12 @@ static int FUNC(sei_buffering_period)(CodedBitstreamContext *ctx, RWContext *rw, ue(seq_parameter_set_id, 0, 31); - sps = h264->sps[current->seq_parameter_set_id]; - if (!sps) { + if (!h264->sps[current->seq_parameter_set_id]) { av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", current->seq_parameter_set_id); return AVERROR_INVALIDDATA; } - h264->active_sps = sps; + h264->active_sps = sps = (const H264RawSPS *)h264->sps[current->seq_parameter_set_id]->data; if (sps->vui.nal_hrd_parameters_present_flag) { for (i = 0; i <= sps->vui.nal_hrd_parameters.cpb_cnt_minus1; i++) { @@ -578,7 +577,7 @@ static int FUNC(sei_pic_timing)(CodedBitstreamContext *ctx, RWContext *rw, } } if (k >= 0) - sps = h264->sps[k]; + sps = (const H264RawSPS *)h264->sps[k]->data; } if (!sps) { av_log(ctx->log_ctx, AV_LOG_ERROR, @@ -1100,21 +1099,19 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, ue(pic_parameter_set_id, 0, 255); - pps = h264->pps[current->pic_parameter_set_id]; - if (!pps) { + if (!h264->pps[current->pic_parameter_set_id]) { av_log(ctx->log_ctx, AV_LOG_ERROR, "PPS id %d not available.\n", current->pic_parameter_set_id); return AVERROR_INVALIDDATA; } - h264->active_pps = pps; + h264->active_pps = pps = (const H264RawPPS *)h264->pps[current->pic_parameter_set_id]->data; - sps = h264->sps[pps->seq_parameter_set_id]; - if (!sps) { + if (!h264->sps[pps->seq_parameter_set_id]) { av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", pps->seq_parameter_set_id); return AVERROR_INVALIDDATA; } - h264->active_sps = sps; + h264->active_sps = sps = (const H264RawSPS *)h264->sps[pps->seq_parameter_set_id]->data; if (sps->separate_colour_plane_flag) u(2, colour_plane_id, 0, 2); diff --git a/libavcodec/cbs_h265.h b/libavcodec/cbs_h265.h index 1b357293ab..4eec99e9a8 100644 --- a/libavcodec/cbs_h265.h +++ b/libavcodec/cbs_h265.h @@ -522,9 +522,9 @@ typedef struct CodedBitstreamH265Context { // All currently available parameter sets. These are updated when // any parameter set NAL unit is read/written with this context. - H265RawVPS *vps[HEVC_MAX_VPS_COUNT]; - H265RawSPS *sps[HEVC_MAX_SPS_COUNT]; - H265RawPPS *pps[HEVC_MAX_PPS_COUNT]; + AVBufferRef *vps[HEVC_MAX_VPS_COUNT]; + AVBufferRef *sps[HEVC_MAX_SPS_COUNT]; + AVBufferRef *pps[HEVC_MAX_PPS_COUNT]; // The currently active parameter sets. These are updated when any // NAL unit refers to the relevant parameter set. These pointers diff --git a/libavcodec/cbs_h265_syntax_template.c b/libavcodec/cbs_h265_syntax_template.c index 9f13061f38..bc9e6ddf08 100644 --- a/libavcodec/cbs_h265_syntax_template.c +++ b/libavcodec/cbs_h265_syntax_template.c @@ -690,11 +690,12 @@ static int FUNC(sps)(CodedBitstreamContext *ctx, RWContext *rw, CHECK(FUNC(nal_unit_header)(ctx, rw, ¤t->nal_unit_header, HEVC_NAL_SPS)); u(4, sps_video_parameter_set_id, 0, 15); - h265->active_vps = vps = h265->vps[current->sps_video_parameter_set_id]; + h265->active_vps = vps = NULL; u(3, sps_max_sub_layers_minus1, 0, HEVC_MAX_SUB_LAYERS - 1); flag(sps_temporal_id_nesting_flag); - if (vps) { + if (h265->vps[current->sps_video_parameter_set_id]) { + h265->active_vps = vps = (const H265RawVPS *)h265->vps[current->sps_video_parameter_set_id]->data; if (vps->vps_max_sub_layers_minus1 > current->sps_max_sub_layers_minus1) { av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid stream: " "sps_max_sub_layers_minus1 (%d) must be less than or equal to " @@ -949,13 +950,13 @@ static int FUNC(pps)(CodedBitstreamContext *ctx, RWContext *rw, ue(pps_pic_parameter_set_id, 0, 63); ue(pps_seq_parameter_set_id, 0, 15); - sps = h265->sps[current->pps_seq_parameter_set_id]; - if (!sps) { + + if (!h265->sps[current->pps_seq_parameter_set_id]) { av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", current->pps_seq_parameter_set_id); return AVERROR_INVALIDDATA; } - h265->active_sps = sps; + h265->active_sps = sps = (const H265RawSPS *)h265->sps[current->pps_seq_parameter_set_id]->data; flag(dependent_slice_segments_enabled_flag); flag(output_flag_present_flag); @@ -1224,21 +1225,19 @@ static int FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw, ue(slice_pic_parameter_set_id, 0, 63); - pps = h265->pps[current->slice_pic_parameter_set_id]; - if (!pps) { + if (!h265->pps[current->slice_pic_parameter_set_id]) { av_log(ctx->log_ctx, AV_LOG_ERROR, "PPS id %d not available.\n", current->slice_pic_parameter_set_id); return AVERROR_INVALIDDATA; } - h265->active_pps = pps; + h265->active_pps = pps = (const H265RawPPS *)h265->pps[current->slice_pic_parameter_set_id]->data; - sps = h265->sps[pps->pps_seq_parameter_set_id]; - if (!sps) { + if (!h265->sps[pps->pps_seq_parameter_set_id]) { av_log(ctx->log_ctx, AV_LOG_ERROR, "SPS id %d not available.\n", pps->pps_seq_parameter_set_id); return AVERROR_INVALIDDATA; } - h265->active_sps = sps; + h265->active_sps = sps = (const H265RawSPS *)h265->sps[pps->pps_seq_parameter_set_id]->data; min_cb_log2_size_y = sps->log2_min_luma_coding_block_size_minus3 + 3; ctb_log2_size_y = min_cb_log2_size_y + sps->log2_diff_max_min_luma_coding_block_size;
This is more in line to how h264_ps handles it. Signed-off-by: James Almer <jamrial@gmail.com> --- Not really going to make much of a difference seeing parameter sets rarely show up in-band even in raw streams, but it's one less memcpy anyway. libavcodec/cbs_h264.h | 4 +-- libavcodec/cbs_h2645.c | 41 ++++++++++++++------------- libavcodec/cbs_h264_syntax_template.c | 19 ++++++------- libavcodec/cbs_h265.h | 6 ++-- libavcodec/cbs_h265_syntax_template.c | 21 +++++++------- 5 files changed, 44 insertions(+), 47 deletions(-)