[FFmpeg-devel] [PATCH] h264: make H264ParamSets sps const
Benoit Fouet
benoit.fouet at free.fr
Tue Jun 21 16:20:03 CEST 2016
Hi,
On 21/06/2016 14:52, Hendrik Leppkes wrote:
> On Tue, Jun 21, 2016 at 2:40 PM, Clément Bœsch <u at pkh.me> wrote:
>> On Tue, Jun 21, 2016 at 02:34:33PM +0200, Benoit Fouet wrote:
>>> Hi,
>>>
>>> Unless I totally missed something, the FIXME in H264ParamSets structure
>>> should be fixed by attached patch.
>>>
>>> --
>>> Ben
>>>
>>> From 28ae10498f81070539bdb8f40236326743350101 Mon Sep 17 00:00:00 2001
>>> From: Benoit Fouet <benoit.fouet at free.fr>
>>> Date: Tue, 21 Jun 2016 14:17:13 +0200
>>> Subject: [PATCH] h264: make H264ParamSets sps const
>>>
>>> ---
>>> libavcodec/h264.h | 3 +--
>>> libavcodec/h264_slice.c | 2 +-
>>> 2 files changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/libavcodec/h264.h b/libavcodec/h264.h
>>> index c4d2921..b809ee5 100644
>>> --- a/libavcodec/h264.h
>>> +++ b/libavcodec/h264.h
>>> @@ -234,8 +234,7 @@ typedef struct H264ParamSets {
>>> AVBufferRef *sps_ref;
>>> /* currently active parameters sets */
>>> const PPS *pps;
>>> - // FIXME this should properly be const
>>> - SPS *sps;
>>> + const SPS *sps;
>>> } H264ParamSets;
>>>
>>> /**
>>> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
>>> index 6e7b940..da7f9dd 100644
>>> --- a/libavcodec/h264_slice.c
>>> +++ b/libavcodec/h264_slice.c
>>> @@ -873,7 +873,7 @@ static enum AVPixelFormat get_pixel_format(H264Context *h, int force_callback)
>>> /* export coded and cropped frame dimensions to AVCodecContext */
>>> static int init_dimensions(H264Context *h)
>>> {
>>> - SPS *sps = h->ps.sps;
>>> + SPS *sps = (SPS*)h->ps.sps_ref->data;
>>> int width = h->width - (sps->crop_right + sps->crop_left);
>>> int height = h->height - (sps->crop_top + sps->crop_bottom);
>>> av_assert0(sps->crop_right + sps->crop_left < (unsigned)h->width);
>> So it's not actually const, right?
>>
> Indeed, the FIXME wasn't just there because someone forgot to write
> "const" in front of it, but because it was used in some parts as
> not-const.
OK, right... Thanks for reminding me of reading the code better before
sending a patch.
As far as I can see, the only place where this constness is not
preserved is in the init_dimensions function (in h264_slice), in a dead
part of the code, as crop is asserted at the beginning of the very same
function.
Please correct me if I've missed other places.
Updated patch attached (which I can very well split in two different
ones if it's (more) correct).
Thanks,
--
Ben
-------------- next part --------------
From cc56a2f787c64ca4051be9b53dd2e4e9b472339c Mon Sep 17 00:00:00 2001
From: Benoit Fouet <benoit.fouet at free.fr>
Date: Tue, 21 Jun 2016 14:17:13 +0200
Subject: [PATCH] h264: make H264ParamSets sps const
Only init_dimension was abusing the data, in a case that is asserted can
never happen.
---
libavcodec/h264.h | 3 +--
libavcodec/h264_parser.c | 2 +-
libavcodec/h264_ps.c | 2 +-
libavcodec/h264_sei.c | 4 ++--
libavcodec/h264_slice.c | 21 ++-------------------
5 files changed, 7 insertions(+), 25 deletions(-)
diff --git a/libavcodec/h264.h b/libavcodec/h264.h
index c4d2921..b809ee5 100644
--- a/libavcodec/h264.h
+++ b/libavcodec/h264.h
@@ -234,8 +234,7 @@ typedef struct H264ParamSets {
AVBufferRef *sps_ref;
/* currently active parameters sets */
const PPS *pps;
- // FIXME this should properly be const
- SPS *sps;
+ const SPS *sps;
} H264ParamSets;
/**
diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 42ad932..76ed2a3 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -373,7 +373,7 @@ static inline int parse_nal_units(AVCodecParserContext *s,
"non-existing SPS %u referenced\n", p->ps.pps->sps_id);
goto fail;
}
- p->ps.sps = (SPS*)p->ps.sps_list[p->ps.pps->sps_id]->data;
+ p->ps.sps = (const SPS*)p->ps.sps_list[p->ps.pps->sps_id]->data;
sps = p->ps.sps;
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index fb05b05..3867918 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -738,7 +738,7 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct
ret = AVERROR_INVALIDDATA;
goto fail;
}
- sps = (SPS*)ps->sps_list[pps->sps_id]->data;
+ sps = (const SPS*)ps->sps_list[pps->sps_id]->data;
if (sps->bit_depth_luma > 14) {
av_log(avctx, AV_LOG_ERROR,
"Invalid luma bit depth=%d\n",
diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
index d0596dc..fedb172 100644
--- a/libavcodec/h264_sei.c
+++ b/libavcodec/h264_sei.c
@@ -276,7 +276,7 @@ static int decode_buffering_period(H264SEIBufferingPeriod *h, GetBitContext *gb,
{
unsigned int sps_id;
int sched_sel_idx;
- SPS *sps;
+ const SPS *sps;
sps_id = get_ue_golomb_31(gb);
if (sps_id > 31 || !ps->sps_list[sps_id]) {
@@ -284,7 +284,7 @@ static int decode_buffering_period(H264SEIBufferingPeriod *h, GetBitContext *gb,
"non-existing SPS %d referenced in buffering period\n", sps_id);
return AVERROR_INVALIDDATA;
}
- sps = (SPS*)ps->sps_list[sps_id]->data;
+ sps = (const SPS*)ps->sps_list[sps_id]->data;
// NOTE: This is really so duplicated in the standard... See H.264, D.1.1
if (sps->nal_hrd_parameters_present_flag) {
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index 6e7b940..219b8ab 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -356,7 +356,7 @@ int ff_h264_update_thread_context(AVCodecContext *dst,
h->ps.sps_ref = av_buffer_ref(h1->ps.sps_ref);
if (!h->ps.sps_ref)
return AVERROR(ENOMEM);
- h->ps.sps = (SPS*)h->ps.sps_ref->data;
+ h->ps.sps = (const SPS*)h->ps.sps_ref->data;
}
if (need_reinit || !inited) {
@@ -873,7 +873,7 @@ static enum AVPixelFormat get_pixel_format(H264Context *h, int force_callback)
/* export coded and cropped frame dimensions to AVCodecContext */
static int init_dimensions(H264Context *h)
{
- SPS *sps = h->ps.sps;
+ const SPS *sps = (const SPS*)h->ps.sps;
int width = h->width - (sps->crop_right + sps->crop_left);
int height = h->height - (sps->crop_top + sps->crop_bottom);
av_assert0(sps->crop_right + sps->crop_left < (unsigned)h->width);
@@ -889,23 +889,6 @@ static int init_dimensions(H264Context *h)
height = h->avctx->height;
}
- if (width <= 0 || height <= 0) {
- av_log(h->avctx, AV_LOG_ERROR, "Invalid cropped dimensions: %dx%d.\n",
- width, height);
- if (h->avctx->err_recognition & AV_EF_EXPLODE)
- return AVERROR_INVALIDDATA;
-
- av_log(h->avctx, AV_LOG_WARNING, "Ignoring cropping information.\n");
- sps->crop_bottom =
- sps->crop_top =
- sps->crop_right =
- sps->crop_left =
- sps->crop = 0;
-
- width = h->width;
- height = h->height;
- }
-
h->avctx->coded_width = h->width;
h->avctx->coded_height = h->height;
h->avctx->width = width;
--
2.9.0.rc2
More information about the ffmpeg-devel
mailing list