Re: [FFmpeg-devel] [PATCH v2] ffplay, avcodec, avformat: Don't initialize before av_packet_ref()

2020-03-27 Thread Andreas Rheinhardt
Anton Khirnov:
> Quoting Andreas Rheinhardt (2020-03-13 14:28:33)
>> It already initializes the packet.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>> Resending because of 3117f47f19d051d47ba29c9b78c2ca525f0fdb45.
>>
>>  fftools/ffplay.c  | 2 +-
>>  libavcodec/qsvdec_h2645.c | 2 +-
>>  libavcodec/qsvdec_other.c | 2 +-
>>  libavformat/fifo.c| 1 -
>>  libavformat/img2enc.c | 8 
>>  libavformat/tee.c | 1 -
>>  6 files changed, 7 insertions(+), 9 deletions(-)
> 
> Generally looks good, but it occurred to me that the semantics of what
> happens on failure in av_packet_ref() is unspecified, which might lead
> to those uninitialized packets now being full of random data, which
> might be dangerous.
> 
> So we might want to specify that on failure dst is reset to a clean
> state and replace the call to av_packet_free_side_data() with
> av_packet_unref().
> 
Applied, thanks.

- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH v2] ffplay, avcodec, avformat: Don't initialize before av_packet_ref()

2020-03-26 Thread Anton Khirnov
Quoting Andreas Rheinhardt (2020-03-13 14:28:33)
> It already initializes the packet.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> Resending because of 3117f47f19d051d47ba29c9b78c2ca525f0fdb45.
> 
>  fftools/ffplay.c  | 2 +-
>  libavcodec/qsvdec_h2645.c | 2 +-
>  libavcodec/qsvdec_other.c | 2 +-
>  libavformat/fifo.c| 1 -
>  libavformat/img2enc.c | 8 
>  libavformat/tee.c | 1 -
>  6 files changed, 7 insertions(+), 9 deletions(-)

Generally looks good, but it occurred to me that the semantics of what
happens on failure in av_packet_ref() is unspecified, which might lead
to those uninitialized packets now being full of random data, which
might be dangerous.

So we might want to specify that on failure dst is reset to a clean
state and replace the call to av_packet_free_side_data() with
av_packet_unref().

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2] ffplay, avcodec, avformat: Don't initialize before av_packet_ref()

2020-03-13 Thread Andreas Rheinhardt
It already initializes the packet.

Signed-off-by: Andreas Rheinhardt 
---
Resending because of 3117f47f19d051d47ba29c9b78c2ca525f0fdb45.

 fftools/ffplay.c  | 2 +-
 libavcodec/qsvdec_h2645.c | 2 +-
 libavcodec/qsvdec_other.c | 2 +-
 libavformat/fifo.c| 1 -
 libavformat/img2enc.c | 8 
 libavformat/tee.c | 1 -
 6 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fftools/ffplay.c b/fftools/ffplay.c
index f6511e4afd..2ed4b22d3e 100644
--- a/fftools/ffplay.c
+++ b/fftools/ffplay.c
@@ -2977,7 +2977,7 @@ static int read_thread(void *arg)
 }
 if (is->queue_attachments_req) {
 if (is->video_st && is->video_st->disposition & 
AV_DISPOSITION_ATTACHED_PIC) {
-AVPacket copy = { 0 };
+AVPacket copy;
 if ((ret = av_packet_ref(, >video_st->attached_pic)) 
< 0)
 goto fail;
 packet_queue_put(>videoq, );
diff --git a/libavcodec/qsvdec_h2645.c b/libavcodec/qsvdec_h2645.c
index 730feed20a..02c41883b6 100644
--- a/libavcodec/qsvdec_h2645.c
+++ b/libavcodec/qsvdec_h2645.c
@@ -125,7 +125,7 @@ static int qsv_decode_frame(AVCodecContext *avctx, void 
*data,
 
 /* buffer the input packet */
 if (avpkt->size) {
-AVPacket input_ref = { 0 };
+AVPacket input_ref;
 
 if (av_fifo_space(s->packet_fifo) < sizeof(input_ref)) {
 ret = av_fifo_realloc2(s->packet_fifo,
diff --git a/libavcodec/qsvdec_other.c b/libavcodec/qsvdec_other.c
index ff2834c20b..b4df76739c 100644
--- a/libavcodec/qsvdec_other.c
+++ b/libavcodec/qsvdec_other.c
@@ -123,7 +123,7 @@ static int qsv_decode_frame(AVCodecContext *avctx, void 
*data,
 
 /* buffer the input packet */
 if (avpkt->size) {
-AVPacket input_ref = { 0 };
+AVPacket input_ref;
 
 if (av_fifo_space(s->packet_fifo) < sizeof(input_ref)) {
 ret = av_fifo_realloc2(s->packet_fifo,
diff --git a/libavformat/fifo.c b/libavformat/fifo.c
index 7b37fff6da..d11dc6626c 100644
--- a/libavformat/fifo.c
+++ b/libavformat/fifo.c
@@ -536,7 +536,6 @@ static int fifo_write_packet(AVFormatContext *avf, AVPacket 
*pkt)
 int ret;
 
 if (pkt) {
-av_init_packet();
 ret = av_packet_ref(,pkt);
 if (ret < 0)
 return ret;
diff --git a/libavformat/img2enc.c b/libavformat/img2enc.c
index a2786ec6f8..b303d38239 100644
--- a/libavformat/img2enc.c
+++ b/libavformat/img2enc.c
@@ -78,7 +78,7 @@ static int write_muxed_file(AVFormatContext *s, AVIOContext 
*pb, AVPacket *pkt)
 VideoMuxData *img = s->priv_data;
 AVCodecParameters *par = s->streams[pkt->stream_index]->codecpar;
 AVStream *st;
-AVPacket pkt2 = {0};
+AVPacket pkt2;
 AVFormatContext *fmt = NULL;
 int ret;
 
@@ -88,8 +88,8 @@ static int write_muxed_file(AVFormatContext *s, AVIOContext 
*pb, AVPacket *pkt)
 return ret;
 st = avformat_new_stream(fmt, NULL);
 if (!st) {
-avformat_free_context(fmt);
-return AVERROR(ENOMEM);
+ret = AVERROR(ENOMEM);
+goto out;
 }
 st->id = pkt->stream_index;
 
@@ -105,8 +105,8 @@ static int write_muxed_file(AVFormatContext *s, AVIOContext 
*pb, AVPacket *pkt)
 (ret = av_interleaved_write_frame(fmt, )) < 0 ||
 (ret = av_write_trailer(fmt))) {}
 
-out:
 av_packet_unref();
+out:
 avformat_free_context(fmt);
 return ret;
 }
diff --git a/libavformat/tee.c b/libavformat/tee.c
index 56669d9d8e..f2b11fcb35 100644
--- a/libavformat/tee.c
+++ b/libavformat/tee.c
@@ -564,7 +564,6 @@ static int tee_write_packet(AVFormatContext *avf, AVPacket 
*pkt)
 if (s2 < 0)
 continue;
 
-memset(, 0, sizeof(AVPacket));
 if ((ret = av_packet_ref(, pkt)) < 0)
 if (!ret_all) {
 ret_all = ret;
-- 
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-requ...@ffmpeg.org with subject "unsubscribe".