On Tue, Jun 21, 2016 at 4:20 PM, Benoit Fouet <benoit.fo...@free.fr> wrote:
> Hi,
>
>
> On 21/06/2016 14:52, Hendrik Leppkes wrote:
>>
>> On Tue, Jun 21, 2016 at 2:40 PM, Clément Bœsch <u...@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.fo...@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.
>
If anything the asserts should probably be removed, because bad files
should never be able to trigger assertions, and the existing check
remain.
- Hendrik
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
