[FFmpeg-devel] [Patch] fix ffprobe crash #3603
Anshul
anshul.ffmpeg at gmail.com
Mon May 12 22:41:14 CEST 2014
On May 13, 2014 12:04:25 AM IST, Stefano Sabatini <stefasab at gmail.com> wrote:
>On date Saturday 2014-05-10 13:44:40 +0530, Anshul encoded:
>> On May 10, 2014 3:45:41 AM IST, Michael Niedermayer
><michaelni at gmx.at> wrote:
>> >On Fri, May 09, 2014 at 04:15:36PM +0530, anshul wrote:
>> >> On 05/09/2014 12:47 PM, Clément Boesch wrote:
>> >> >On Fri, May 09, 2014 at 09:15:53AM +0200, Clément Boesch wrote:
>> >> >>On Fri, May 09, 2014 at 12:33:36PM +0530, anshul wrote:
>> >> >>>On 05/09/2014 06:15 AM, Michael Niedermayer wrote:
>> >> >>>>>this patch consider all three things.
>> >> >>>>did you intend to attach anoter patch ?
>> >> >>>>iam asking as there was no patch attached to your last mail
>> >> >>>>
>> >> >>>>
>> >> >>>yes, I am sorry for that.
>> >> >>>
>> >> >>>-Anshul
>> >> >>> From 3ee1e369b42f0baa29706739f0b328615d20ebee Mon Sep 17
>00:00:00
>> >2001
>> >> >>>From: Anshul Maheshwari <er.anshul.maheshwari at gmail.com>
>> >> >>>Date: Thu, 8 May 2014 12:23:26 +0530
>> >> >>>Subject: [PATCH] ffprobe: fix crash because of new streams
>> >occuring
>> >> >>>
>> >> >>>Fix ticket #3603
>> >> >>>---
>> >> >>> ffprobe.c | 19 ++++++++++++++-----
>> >> >>> 1 file changed, 14 insertions(+), 5 deletions(-)
>> >> >>>
>> >> >>>diff --git a/ffprobe.c b/ffprobe.c
>> >> >>>index c6e0469..5d6bf01 100644
>> >> >>>--- a/ffprobe.c
>> >> >>>+++ b/ffprobe.c
>> >> >>>@@ -191,6 +191,7 @@ static const char unit_hertz_str[]
>=
>> >"Hz" ;
>> >> >>> static const char unit_byte_str[] = "byte" ;
>> >> >>> static const char unit_bit_per_second_str[] = "bit/s";
>> >> >>>+static int nb_streams;
>> >> >>> static uint64_t *nb_streams_packets;
>> >> >>> static uint64_t *nb_streams_frames;
>> >> >>> static int *selected_streams;
>> >> >>>@@ -1893,6 +1894,12 @@ static int
>> >read_interval_packets(WriterContext *w, AVFormatContext *fmt_ctx,
>> >> >>> goto end;
>> >> >>> }
>> >> >>> while (!av_read_frame(fmt_ctx, &pkt)) {
>> >> >>>+ if(fmt_ctx->nb_streams >= nb_streams) {
>> >> >>>+ nb_streams_frames =
>> >av_realloc(nb_streams_frames,fmt_ctx->nb_streams*
>> >sizeof(*nb_streams_frames));
>> >> >>>+ nb_streams_packets =
>> >av_realloc(nb_streams_packets,fmt_ctx->nb_streams*
>> >sizeof(*nb_streams_packets));
>> >> >>>+ selected_streams =
>> >av_realloc(selected_streams,fmt_ctx->nb_streams*
>> >sizeof(*selected_streams));
>> >> >>space after ,
>> >> >>space before *
>> >> >for the mult obviously
>> >> >
>> >> >And speaking of this, you should use av_realloc_array for the
>> >overflow
>> >> >check.
>> >> >
>> >> >>space before (
>> >> >>
>> >> >for the if
>> >> >
>> >> >[...]
>> >> >
>> >> >
>> >> >
>> >> >_______________________________________________
>> >> >ffmpeg-devel mailing list
>> >> >ffmpeg-devel at ffmpeg.org
>> >> >http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> >> Please ignore previous patch, i don't know what is wrong with me.
>> >> Again attached new patch for fixing this crash
>> >> -Anshul
>> >
>> >> ffprobe.c | 40 ++++++++++++++++++++++++++++++++++------
>> >> 1 file changed, 34 insertions(+), 6 deletions(-)
>> >> cefae455261a61fba6796d0dc5d261349ee44385
>> >0001-ffprobe-fix-crash-because-of-new-streams-occuring.patch
>> >> From 12685c54a34b6ab5fcbc70cf86c4248dede61bdc Mon Sep 17 00:00:00
>> >2001
>> >> From: Anshul Maheshwari <er.anshul.maheshwari at gmail.com>
>> >> Date: Fri, 9 May 2014 16:12:28 +0530
>> >> Subject: [PATCH] ffprobe: fix crash because of new streams
>occuring
>> >>
>> >> Fix ticket #3603
>> >> ---
>> >> ffprobe.c | 40 ++++++++++++++++++++++++++++++++++------
>> >> 1 file changed, 34 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/ffprobe.c b/ffprobe.c
>> >> index c6e0469..b9528e5 100644
>> >> --- a/ffprobe.c
>> >> +++ b/ffprobe.c
>> >> @@ -191,9 +191,10 @@ static const char unit_hertz_str[] =
>> >"Hz" ;
>> >> static const char unit_byte_str[] = "byte" ;
>> >> static const char unit_bit_per_second_str[] = "bit/s";
>> >>
>> >> +static int nb_streams;
>> >
>> >> -static uint64_t *nb_streams_packets;
>> >> -static uint64_t *nb_streams_frames;
>> >> -static int *selected_streams;
>> >> +static uint64_t *nb_streams_packets = NULL;
>> >> +static uint64_t *nb_streams_frames = NULL;
>> >> +static int *selected_streams = NULL;
>> >
>> >thats unrelated
>> >statics are already initialized to 0 by default
>> >
>> >
>>
>> Done
>> >>
>> >> static void ffprobe_cleanup(int ret)
>> >> {
>> >> @@ -1893,6 +1894,25 @@ static int
>read_interval_packets(WriterContext
>> >*w, AVFormatContext *fmt_ctx,
>> >> goto end;
>> >> }
>> >> while (!av_read_frame(fmt_ctx, &pkt)) {
>> >> + if (fmt_ctx->nb_streams > nb_streams) {
>> >> + ret = av_reallocp_array(&nb_streams_frames,
>> >fmt_ctx->nb_streams, sizeof(*nb_streams_frames));
>> >> + if(ret)
>> >> + goto end;
>> >> + ret = av_reallocp_array(&nb_streams_packets,
>> >fmt_ctx->nb_streams, sizeof(*nb_streams_packets));
>> >> + if(ret)
>> >> + goto end;
>> >> + ret = av_reallocp_array(&selected_streams,
>> >fmt_ctx->nb_streams, sizeof(*selected_streams));
>> >> + if(ret)
>> >> + goto end;
>> >> + memset(nb_streams_frames + nb_streams, 0,
>> >> + (fmt_ctx->nb_streams - nb_streams) *
>> >sizeof(*nb_streams_frames));
>> >> + memset(nb_streams_packets + nb_streams, 0,
>> >> + (fmt_ctx->nb_streams - nb_streams) *
>> >sizeof(*nb_streams_packets));
>> >> + memset(selected_streams + nb_streams, 0,
>> >> + (fmt_ctx->nb_streams - nb_streams) *
>> >sizeof(*selected_streams));
>> >> + nb_streams = fmt_ctx->nb_streams;
>> >> + }
>> >
>> >> +#if 0
>> >> if (selected_streams[pkt.stream_index]) {
>> >> AVRational tb =
>> >fmt_ctx->streams[pkt.stream_index]->time_base;
>> >>
>> >> @@ -1928,6 +1948,7 @@ static int
>read_interval_packets(WriterContext
>> >*w, AVFormatContext *fmt_ctx,
>> >> }
>> >> }
>> >> av_free_packet(&pkt);
>> >> +#endif
>> >
>> >why is this code left in there and disabled ?
>> >
>> >[...]
>>
>> Sorry for that I was debugging by ifdefs and left one there only,
>thats not required.
>>
>> -Anshul
>>
>> --
>> Sent from my Android device with K-9 Mail. Please excuse my brevity.
>
>> From 59c61b155ce290b80da4c50db84d0ecd6a139180 Mon Sep 17 00:00:00
>2001
>> From: Anshul <er.anshul.maheshwari at gmail.com>
>> Date: Sat, 10 May 2014 13:23:11 +0530
>> Subject: [PATCH] ffprobe: fix crash because of new streams occuring
>>
>> Fix ticket #3603
>> ---
>> ffprobe.c | 32 +++++++++++++++++++++++++++++---
>> 1 file changed, 29 insertions(+), 3 deletions(-)
>>
>> diff --git a/ffprobe.c b/ffprobe.c
>> index c6e0469..b3d7aed 100644
>> --- a/ffprobe.c
>> +++ b/ffprobe.c
>> @@ -191,6 +191,7 @@ static const char unit_hertz_str[] =
>"Hz" ;
>> static const char unit_byte_str[] = "byte" ;
>> static const char unit_bit_per_second_str[] = "bit/s";
>>
>> +static int nb_streams;
>> static uint64_t *nb_streams_packets;
>> static uint64_t *nb_streams_frames;
>> static int *selected_streams;
>> @@ -1893,6 +1894,24 @@ static int read_interval_packets(WriterContext
>*w, AVFormatContext *fmt_ctx,
>> goto end;
>> }
>> while (!av_read_frame(fmt_ctx, &pkt)) {
>> + if (fmt_ctx->nb_streams > nb_streams) {
>> + ret = av_reallocp_array(&nb_streams_frames,
>fmt_ctx->nb_streams, sizeof(*nb_streams_frames));
>
>> + if(ret)
>> + goto end;
>
>if (ret < 0)
>
>here and below
Done
>
>> + ret = av_reallocp_array(&nb_streams_packets,
>fmt_ctx->nb_streams, sizeof(*nb_streams_packets));
>> + if(ret)
>> + goto end;
>> + ret = av_reallocp_array(&selected_streams,
>fmt_ctx->nb_streams, sizeof(*selected_streams));
>> + if(ret)
>> + goto end;
>> + memset(nb_streams_frames + nb_streams, 0,
>> + (fmt_ctx->nb_streams - nb_streams) *
>sizeof(*nb_streams_frames));
>> + memset(nb_streams_packets + nb_streams, 0,
>> + (fmt_ctx->nb_streams - nb_streams) *
>sizeof(*nb_streams_packets));
>> + memset(selected_streams + nb_streams, 0,
>> + (fmt_ctx->nb_streams - nb_streams) *
>sizeof(*selected_streams));
>
>nit: you can probably factorize this with a macro
>
I have tried to factorize.
>> + nb_streams = fmt_ctx->nb_streams;
>> + }
>> if (selected_streams[pkt.stream_index]) {
>> AVRational tb =
>fmt_ctx->streams[pkt.stream_index]->time_base;
>>
>> @@ -2373,10 +2392,17 @@ static int probe_file(WriterContext *wctx,
>const char *filename)
>> return ret;
>>
>> #define CHECK_END if (ret < 0) goto end
>> + nb_streams = fmt_ctx->nb_streams;
>> + if(av_reallocp_array(&nb_streams_frames, fmt_ctx->nb_streams,
>sizeof(*nb_streams_frames)))
>> + CHECK_END;
>
>are you missing the ret assignment?
>ret = av_reallocp_array(...);
>CHECK_END;
>
Yes i was.
>> + if(av_reallocp_array(&nb_streams_packets, fmt_ctx->nb_streams,
>sizeof(*nb_streams_packets)))
>> + CHECK_END;
>> + if(av_reallocp_array(&selected_streams, fmt_ctx->nb_streams,
>sizeof(*selected_streams)))
>> + CHECK_END;
>>
>> - nb_streams_frames = av_calloc(fmt_ctx->nb_streams,
>sizeof(*nb_streams_frames));
>> - nb_streams_packets = av_calloc(fmt_ctx->nb_streams,
>sizeof(*nb_streams_packets));
>> - selected_streams = av_calloc(fmt_ctx->nb_streams,
>sizeof(*selected_streams));
>> + memset(nb_streams_frames, 0, nb_streams *
>sizeof(*nb_streams_frames));
>> + memset(nb_streams_packets, 0, nb_streams *
>sizeof(*nb_streams_packets));
>> + memset(selected_streams, 0, nb_streams *
>sizeof(*selected_streams));
>
>[Will probably fix it and push it myself later if I'm still alive at
>the end of the day.]
I was already dead, so could not pull the git, and download that sample to test this latest one.
-Anshul
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ffprobe-fix-crash-because-of-new-streams-occuring.patch
Type: application/octet-stream
Size: 2982 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140513/f4ced32b/attachment.obj>
More information about the ffmpeg-devel
mailing list