[FFmpeg-devel] [PATCH 2/2 v2] avdevice/decklink_dec: use av_packet_add_side_data()

[James Almer]

On 10/2/2017 2:20 PM, Marton Balint wrote:
> 
> 
> On Mon, 2 Oct 2017, James Almer wrote:
> 
>> It uses the existing buffer instead of allocating a new one.
>>
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>> Untested. No changes since last version.
>>
>> libavdevice/decklink_dec.cpp | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
>> index d8c624aa5d..a172110bdd 100644
>> --- a/libavdevice/decklink_dec.cpp
>> +++ b/libavdevice/decklink_dec.cpp
>> @@ -390,10 +390,8 @@ uint8_t *get_metadata(AVFormatContext *avctx,
>> uint16_t *buf, size_t width,
>>             clear_parity_bits(buf, len);
>>             data = vanc_to_cc(avctx, buf, width, data_len);
>>             if (data) {
>> -                uint8_t *pkt_cc = av_packet_new_side_data(pkt,
>> AV_PKT_DATA_A53_CC, data_len);
>> -                if (pkt_cc)
>> -                    memcpy(pkt_cc, data, data_len);
>> -                av_free(data);
>> +                if (av_packet_add_side_data(pkt, AV_PKT_DATA_A53_CC,
>> data, data_len) < 0)
>> +                    av_free(data);
>>             }
>>         } else {
>>             av_log(avctx, AV_LOG_DEBUG, "Unknown meta data DID =
>> 0x%.2x SDID = 0x%.2x\n",
> 
> I can't test this one, but LGTM.
> 
> On a side note, maybe we should clarify if AV_INPUT_BUFFER_PADDING_SIZE
> is required on side data or not, the docs of av_packet_add_side_data
> does not mention it but av_packet_new_side_data adds the padding.

Ideally, av_packet_add_side_data() would have been defined the same as
av_packet_from_data(), requiring the user to make sure the buffers
passed to it are padded, but changing that now is probably an API break.
And since the doxy for av_packet_new_side_data() does not mention the
buffers it creates are padded either, user code should probably expect
them to not be as such.

There are some efforts right now in trying to replace all the assorted
side data APIs (packet, stream, frame) with a single reusable generic
API, where we should make sure this is well defined.

Pushed, thanks.

> 
> Thanks,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel