| Message ID | 20171002162638.5932-2-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
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@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. Thanks, Marton
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@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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
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",
It uses the existing buffer instead of allocating a new one. Signed-off-by: James Almer <jamrial@gmail.com> --- Untested. No changes since last version. libavdevice/decklink_dec.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)