From: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> To: ffmpeg-devel@ffmpeg.org Subject: Re: [FFmpeg-devel] [PATCH] avformat/vpk: fix divide by zero Date: Sat, 10 Aug 2024 11:25:38 +0200 Message-ID: <DU0P250MB0747EC57F8AECD18700FBE008FBB2@DU0P250MB0747.EURP250.PROD.OUTLOOK.COM> (raw) In-Reply-To: <CABPLASQdGJYmxRNx6imnadzcr1kqkN_9Bu+gF_SxTzdMCiOmFw@mail.gmail.com> Kacper Michajlow: > On Fri, 9 Aug 2024 at 22:51, Michael Niedermayer <michael@niedermayer.cc> wrote: >> >> On Wed, Aug 07, 2024 at 03:42:46PM +0200, Kacper Michajłow wrote: >>> Can happen after calling avformat_find_stream_info() when the codec >>> fails to open, but return value is 0 and subsequent uses of this context >>> have zero value in channel number. >>> >>> Found by OSS-Fuzz. >>> >>> Signed-off-by: Kacper Michajłow <kasper93@gmail.com> >>> --- >>> libavformat/vpk.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/libavformat/vpk.c b/libavformat/vpk.c >>> index 001ad33555..aa98ef2dd4 100644 >>> --- a/libavformat/vpk.c >>> +++ b/libavformat/vpk.c >>> @@ -86,6 +86,8 @@ static int vpk_read_packet(AVFormatContext *s, AVPacket *pkt) >>> >>> vpk->current_block++; >>> if (vpk->current_block == vpk->block_count) { >>> + if (par->ch_layout.nb_channels <= 0) >>> + return AVERROR_INVALIDDATA; >>> unsigned size = vpk->last_block_size / par->ch_layout.nb_channels; >>> unsigned skip = (par->block_align - vpk->last_block_size) / par->ch_layout.nb_channels; >>> uint64_t pos = avio_tell(s->pb); >> >> iam not sure if a parser or other should replace a valid set of >> parameters by an invalid >> (this patch implies that such a action occured) >> >> can you explain more detailedly by what and why channels is set to 0 ? >> > > You are right, it might be better to improve this to not override the > params. Let me explain what happens, I didn't read through the whole > avformat_find_stream_info() to know what would be the best approach > yet. I will try to look at it, but if you have immediate ideas, that > would be nice. > > 1. avformat_open_input() sets nb_channels to 108 > > 2. Just after that we call avformat_find_stream_info(avfc, NULL); this > returns 0 (success), but as a result it overrides params already > present in the context. > log for reference, during the find stream info call > [ffmpeg/demuxer] vpk: Before avformat_find_stream_info() pos: > 538976288 bytes read:21 seeks:1 nb_streams:1 > [ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info > [lavf] mp_seek(0x512000018090, 0, size) > [lavf] 0=mp_read(0x512000018090, 0x7fe4c7ce8800, 50000000), pos: > 538976288, eof:1 > [lavf] 0=mp_read(0x512000018090, 0x52d00000a400, 32768), pos: 538976288, eof:1 > [ffmpeg/audio] adpcm_psx: Decoder requires channel layout to be set > [ffmpeg/demuxer] vpk: Failed to open codec in avformat_find_stream_info > [lavf] mp_seek(0x512000018090, 0, size) > [lavf] mp_seek(0x512000018090, 0, size) > [lavf] mp_seek(0x512000018090, 0, size) > [ffmpeg/demuxer] vpk: stream 0: start_time: NOPTS duration: 0.069852 > [ffmpeg/demuxer] vpk: format: start_time: NOPTS duration: 0.069852 > (estimate from stream) bitrate=2 kb/s > [ffmpeg/demuxer] vpk: Could not find codec parameters for stream 0 > (Audio: adpcm_psx, 538976288 Hz, 0 channels): unspecified sample > format > [ffmpeg/demuxer] Consider increasing the value for the > 'analyzeduration' (0) and 'probesize' (5000000) options > [ffmpeg/demuxer] vpk: After avformat_find_stream_info() pos: 538976288 > bytes read:21 seeks:1 frames:0 > > 3. the nb_channels value is cleared in avformat_find_stream_info() -> > avcodec_parameters_from_context() -> codec_parameters_reset() and > remains 0. This seems like the error: Why is AVCodecParameters being set from an AVCodecContext if the codec could not be successfully opened? > > 4. as we can see there were errors, but it still returns success, so we proceed. > > 5. on the next av_seek_frame() which goes to vpk_read_packet() it will > fail because nb_channels is 0 at this point. > > Sorry for only a high level overview, but at this moment, I'm not sure > how exactly it is supposed to work. I thought it might be intended to > override headers parameters later if we know better later on, that's > why my initial patch only tackled this case. > _______________________________________________ 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".
next prev parent reply other threads:[~2024-08-10 9:25 UTC|newest] Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-08-07 13:42 Kacper Michajłow 2024-08-09 20:42 ` Michael Niedermayer 2024-08-09 23:51 ` Kacper Michajlow 2024-08-10 9:25 ` Andreas Rheinhardt [this message] 2024-08-10 16:49 ` Kacper Michajlow
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=DU0P250MB0747EC57F8AECD18700FBE008FBB2@DU0P250MB0747.EURP250.PROD.OUTLOOK.COM \ --to=andreas.rheinhardt@outlook.com \ --cc=ffmpeg-devel@ffmpeg.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel This inbox may be cloned and mirrored by anyone: git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \ ffmpegdev@gitmailbox.com public-inbox-index ffmpegdev Example config snippet for mirrors. AGPL code for this site: git clone https://public-inbox.org/public-inbox.git