On 5/8/2018 7:17 PM, Mark Thompson wrote:
> On 08/05/18 21:48, James Almer wrote:
>> There's no gain from using AVBufferRef for these, as no copies or
>> references are ever made.
>>
>> Signed-off-by: James Almer <jamr...@gmail.com>
>> ---
>> There is however a raw copy of the struct storing these buffers,
>> which is dangerous and fragile.
>> This patch is in preparation to change how the above is handled.
>>
>> libavcodec/cbs_h264.h | 1 -
>> libavcodec/cbs_h2645.c | 13 ++++++-------
>> libavcodec/cbs_h265.h | 1 -
>> 3 files changed, 6 insertions(+), 9 deletions(-)
>>
>> diff --git a/libavcodec/cbs_h264.h b/libavcodec/cbs_h264.h
>> index 2219d9da8d..becea3adfe 100644
>> --- a/libavcodec/cbs_h264.h
>> +++ b/libavcodec/cbs_h264.h
>> @@ -197,7 +197,6 @@ typedef struct H264RawPPS {
>> uint16_t pic_size_in_map_units_minus1;
>>
>> uint8_t *slice_group_id;
>> - AVBufferRef *slice_group_id_ref;
>>
>> uint8_t num_ref_idx_l0_default_active_minus1;
>> uint8_t num_ref_idx_l1_default_active_minus1;
>> diff --git a/libavcodec/cbs_h2645.c b/libavcodec/cbs_h2645.c
>> index 64a1a2d1ee..580ca09f63 100644
>> --- a/libavcodec/cbs_h2645.c
>> +++ b/libavcodec/cbs_h2645.c
>> @@ -318,10 +318,9 @@ static int cbs_h2645_read_more_rbsp_data(GetBitContext
>> *gbc)
>> #define byte_alignment(rw) (get_bits_count(rw) % 8)
>>
>> #define allocate(name, size) do { \
>> - name ## _ref = av_buffer_allocz(size); \
>> - if (!name ## _ref) \
>> + name = av_mallocz(size); \
>> + if (!name) \
>> return AVERROR(ENOMEM); \
>> - name = name ## _ref->data; \
>> } while (0)
>
> This breaks other users of this macro (H.264 SEI).
>
> The reason for using the bufferref here is not really that you might want to
> make more references to it. Rather, it is for the alloc/free properties
> which give control to the user - for example, they can set one of these
> pointers to some internal static buffer they hold while setting _ref to null,
> and the free code still does the right thing (i.e. nothing).
>
> I don't think that argument will necessarily apply to any of the values
> changed here - I doubt anyone will ever touch the FMO slice_group_id, and the
> H.265 PS extension data bits will need to have defined meanings (and
> therefore moved out of the "unknown future stuff" case) before they gets
> used. Still, I'm not sure how you've gained anything - since the PS objects
> are refcounted in the following patch, how does this change actually help?
Removing the overhead of having these be AVBufferRef instead of a simple
malloc'ed array, since at least after a quick glance i couldn't find any
reason for it.
If you think keeping them as AVBufferRef is better/future proof then we
can drop this. The next patch works around the issue of memcpy'ing them
anyway.
>
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
