[PATCH 1/5] avdevice/lavfi: Use av_packet_move_ref() for packet ownership transfer
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavdevice/lavfi.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index c949ff7e12..2a95cb013c 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -392,10 +392,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) int size = 0; if (lavfi->subcc_packet.size) { - *pkt = lavfi->subcc_packet; - av_init_packet(&lavfi->subcc_packet); - lavfi->subcc_packet.size = 0; - lavfi->subcc_packet.data = NULL; + av_packet_move_ref(pkt, &lavfi->subcc_packet); return pkt->size; } -- 2.20.1
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavdevice/lavfi.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index 2a95cb013c..5e814eada8 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -362,16 +362,12 @@ static int create_subcc_packet(AVFormatContext *avctx, AVFrame *frame, { LavfiContext *lavfi = avctx->priv_data; AVFrameSideData *sd; - int stream_idx, i, ret; + int stream_idx, ret; if ((stream_idx = lavfi->sink_stream_subcc_map[sink_idx]) < 0) return 0; - for (i = 0; i < frame->nb_side_data; i++) - if (frame->side_data[i]->type == AV_FRAME_DATA_A53_CC) - break; - if (i >= frame->nb_side_data) + if (!(sd = av_frame_get_side_data(frame, AV_FRAME_DATA_A53_CC))) return 0; - sd = frame->side_data[i]; if ((ret = av_new_packet(&lavfi->subcc_packet, sd->size)) < 0) return ret; memcpy(lavfi->subcc_packet.data, sd->data, sd->size); -- 2.20.1
On 8/30/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavdevice/lavfi.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)
probably ok
diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index 2a95cb013c..5e814eada8 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -362,16 +362,12 @@ static int create_subcc_packet(AVFormatContext *avctx, AVFrame *frame, { LavfiContext *lavfi = avctx->priv_data; AVFrameSideData *sd; - int stream_idx, i, ret; + int stream_idx, ret;
if ((stream_idx = lavfi->sink_stream_subcc_map[sink_idx]) < 0) return 0; - for (i = 0; i < frame->nb_side_data; i++) - if (frame->side_data[i]->type == AV_FRAME_DATA_A53_CC) - break; - if (i >= frame->nb_side_data) + if (!(sd = av_frame_get_side_data(frame, AV_FRAME_DATA_A53_CC))) return 0; - sd = frame->side_data[i]; if ((ret = av_new_packet(&lavfi->subcc_packet, sd->size)) < 0) return ret; memcpy(lavfi->subcc_packet.data, sd->data, sd->size); -- 2.20.1
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- av_packet_pack_dictionary() returns NULL in case the dictionary's count is zero; but given that the dict API does not return such dicts at all, I have not added any check for this. libavdevice/lavfi.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index 5e814eada8..bafc3275c9 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -444,25 +444,16 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) frame_metadata = frame->metadata; if (frame_metadata) { - uint8_t *metadata; - AVDictionaryEntry *e = NULL; - AVBPrint meta_buf; - - av_bprint_init(&meta_buf, 0, AV_BPRINT_SIZE_UNLIMITED); - while ((e = av_dict_get(frame_metadata, "", e, AV_DICT_IGNORE_SUFFIX))) { - av_bprintf(&meta_buf, "%s", e->key); - av_bprint_chars(&meta_buf, '\0', 1); - av_bprintf(&meta_buf, "%s", e->value); - av_bprint_chars(&meta_buf, '\0', 1); - } - if (!av_bprint_is_complete(&meta_buf) || - !(metadata = av_packet_new_side_data(pkt, AV_PKT_DATA_STRINGS_METADATA, - meta_buf.len))) { - av_bprint_finalize(&meta_buf, NULL); + int size; + uint8_t *metadata = av_packet_pack_dictionary(frame_metadata, &size); + + if (!metadata) return AVERROR(ENOMEM); + if ((ret = av_packet_add_side_data(pkt, AV_PKT_DATA_STRINGS_METADATA, + metadata, size)) < 0) { + av_freep(&metadata); + return ret; } - memcpy(metadata, meta_buf.str, meta_buf.len); - av_bprint_finalize(&meta_buf, NULL); } if ((ret = create_subcc_packet(avctx, frame, min_pts_sink_idx)) < 0) { -- 2.20.1
Andreas Rheinhardt:
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- av_packet_pack_dictionary() returns NULL in case the dictionary's count is zero; but given that the dict API does not return such dicts at all, I have not added any check for this.
libavdevice/lavfi.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index 5e814eada8..bafc3275c9 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -444,25 +444,16 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt)
frame_metadata = frame->metadata; if (frame_metadata) { - uint8_t *metadata; - AVDictionaryEntry *e = NULL; - AVBPrint meta_buf; - - av_bprint_init(&meta_buf, 0, AV_BPRINT_SIZE_UNLIMITED); - while ((e = av_dict_get(frame_metadata, "", e, AV_DICT_IGNORE_SUFFIX))) { - av_bprintf(&meta_buf, "%s", e->key); - av_bprint_chars(&meta_buf, '\0', 1); - av_bprintf(&meta_buf, "%s", e->value); - av_bprint_chars(&meta_buf, '\0', 1); - } - if (!av_bprint_is_complete(&meta_buf) || - !(metadata = av_packet_new_side_data(pkt, AV_PKT_DATA_STRINGS_METADATA, - meta_buf.len))) { - av_bprint_finalize(&meta_buf, NULL); + int size; + uint8_t *metadata = av_packet_pack_dictionary(frame_metadata, &size); + + if (!metadata) return AVERROR(ENOMEM); + if ((ret = av_packet_add_side_data(pkt, AV_PKT_DATA_STRINGS_METADATA, + metadata, size)) < 0) { + av_freep(&metadata); + return ret; } - memcpy(metadata, meta_buf.str, meta_buf.len); - av_bprint_finalize(&meta_buf, NULL); }
if ((ret = create_subcc_packet(avctx, frame, min_pts_sink_idx)) < 0) {
Will apply the two outstanding patches (i.e. this and 4/5 [1]) of this patchset tomorrow unless there are objections. - Andreas [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-August/268930.html
Since bae8844e35147f92e612a9e0b44e939a293e5bc9, the AVPacket that is intended to be used to return the demuxed packet is automatically unreferenced when the demuxer returns an error. This makes an av_packet_unref() in the lavfi demuxer redundant. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavdevice/lavfi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index bafc3275c9..2b4e9290dc 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -458,7 +458,6 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) if ((ret = create_subcc_packet(avctx, frame, min_pts_sink_idx)) < 0) { av_frame_unref(frame); - av_packet_unref(pkt); return ret; } -- 2.20.1
av_new_packet() already sets the size. And if the packet is not allocated by av_new_packet() (which seems to be impossible atm), both pkt->size as well as size are 0, so setting it again is unnecessary in this scenario, too. Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavdevice/lavfi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index 2b4e9290dc..4f05a1542e 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -464,7 +464,6 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) pkt->stream_index = stream_idx; pkt->pts = frame->pts; pkt->pos = frame->pkt_pos; - pkt->size = size; av_frame_unref(frame); return size; } -- 2.20.1
On 8/30/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
av_new_packet() already sets the size. And if the packet is not allocated by av_new_packet() (which seems to be impossible atm), both pkt->size as well as size are 0, so setting it again is unnecessary in this scenario, too.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavdevice/lavfi.c | 1 - 1 file changed, 1 deletion(-)
LGTM
diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index 2b4e9290dc..4f05a1542e 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -464,7 +464,6 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) pkt->stream_index = stream_idx; pkt->pts = frame->pts; pkt->pos = frame->pkt_pos; - pkt->size = size; av_frame_unref(frame); return size; } -- 2.20.1
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 8/30/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavdevice/lavfi.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
LGTM if tested. I think this is covered by FATE.
diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index c949ff7e12..2a95cb013c 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -392,10 +392,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) int size = 0;
if (lavfi->subcc_packet.size) { - *pkt = lavfi->subcc_packet; - av_init_packet(&lavfi->subcc_packet); - lavfi->subcc_packet.size = 0; - lavfi->subcc_packet.data = NULL; + av_packet_move_ref(pkt, &lavfi->subcc_packet); return pkt->size; }
-- 2.20.1
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Paul B Mahol:
On 8/30/20, Andreas Rheinhardt <andreas.rheinhardt@gmail.com> wrote:
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com> --- libavdevice/lavfi.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
LGTM if tested.
I think this is covered by FATE.
It is, see [1]. All patches of this patchset except 4/5 are covered by it. - Andreas [1]: http://coverage.ffmpeg.org/index.src_libavdevice_lavfi.c.html
diff --git a/libavdevice/lavfi.c b/libavdevice/lavfi.c index c949ff7e12..2a95cb013c 100644 --- a/libavdevice/lavfi.c +++ b/libavdevice/lavfi.c @@ -392,10 +392,7 @@ static int lavfi_read_packet(AVFormatContext *avctx, AVPacket *pkt) int size = 0;
if (lavfi->subcc_packet.size) { - *pkt = lavfi->subcc_packet; - av_init_packet(&lavfi->subcc_packet); - lavfi->subcc_packet.size = 0; - lavfi->subcc_packet.data = NULL; + av_packet_move_ref(pkt, &lavfi->subcc_packet); return pkt->size; }
-- 2.20.1
participants (2)
-
Andreas Rheinhardt -
Paul B Mahol