| Message ID | 20210831204547.25082-1-jl@conductive.de |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/wmaprodec: skip foreign XMA packets | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
What samples this fixes?
Hello,
On 2021-08-31 22:53, Paul B Mahol wrote:
> What samples this fixes?
No specific sample, it depends on the way the decoder is invoked.
It fixes using the xma1/2 decoder directly on a memory buffer and not
wanting to decode all channels in the stream.
Say the stream has 6 channels and one wants to only decode 2.
Previously this was not possible due to the skip logic of the channel
interleaving.
This should also make the decoder more robust because in theory it is
possible to have an xma stream which has chunks of 2kB junk data
(packets) which are expected to be skipped due to the skip_packets
numbers.
I haven't seen such streams though.
On Tue, Aug 31, 2021 at 11:10 PM Joel Linn <jl@conductive.de> wrote: > Hello, > > On 2021-08-31 22:53, Paul B Mahol wrote: > > What samples this fixes? > > No specific sample, it depends on the way the decoder is invoked. > > It fixes using the xma1/2 decoder directly on a memory buffer and not > wanting to decode all channels in the stream. > Say the stream has 6 channels and one wants to only decode 2. > Previously this was not possible due to the skip logic of the channel > interleaving. > > This should also make the decoder more robust because in theory it is > possible to have an xma stream which has chunks of 2kB junk data > (packets) which are expected to be skipped due to the skip_packets > numbers. > I haven't seen such streams though. > Am I correct you already use XMA parser?
On 2021-08-31 23:13, Paul B Mahol wrote: > On Tue, Aug 31, 2021 at 11:10 PM Joel Linn <jl@conductive.de> wrote: > >> Hello, >> >> On 2021-08-31 22:53, Paul B Mahol wrote: >> > What samples this fixes? >> >> No specific sample, it depends on the way the decoder is invoked. >> >> It fixes using the xma1/2 decoder directly on a memory buffer and not >> wanting to decode all channels in the stream. >> Say the stream has 6 channels and one wants to only decode 2. >> Previously this was not possible due to the skip logic of the channel >> interleaving. >> >> This should also make the decoder more robust because in theory it is >> possible to have an xma stream which has chunks of 2kB junk data >> (packets) which are expected to be skipped due to the skip_packets >> numbers. >> I haven't seen such streams though. >> > > Am I correct you already use XMA parser? In my case I was working with raw stream buffers, the XMA format (RIFF) headers are invisible to me. The format is parsed and processed by guest code (inside xenia emulator). ffmpeg replaces a hardware decoder dsp in this case.
On Tue, Aug 31, 2021 at 11:19 PM Joel Linn <jl@conductive.de> wrote: > On 2021-08-31 23:13, Paul B Mahol wrote: > > On Tue, Aug 31, 2021 at 11:10 PM Joel Linn <jl@conductive.de> wrote: > > > >> Hello, > >> > >> On 2021-08-31 22:53, Paul B Mahol wrote: > >> > What samples this fixes? > >> > >> No specific sample, it depends on the way the decoder is invoked. > >> > >> It fixes using the xma1/2 decoder directly on a memory buffer and not > >> wanting to decode all channels in the stream. > >> Say the stream has 6 channels and one wants to only decode 2. > >> Previously this was not possible due to the skip logic of the channel > >> interleaving. > >> > >> This should also make the decoder more robust because in theory it is > >> possible to have an xma stream which has chunks of 2kB junk data > >> (packets) which are expected to be skipped due to the skip_packets > >> numbers. > >> I haven't seen such streams though. > >> > > > > Am I correct you already use XMA parser? > > In my case I was working with raw stream buffers, the XMA format (RIFF) > headers are invisible to me. > The format is parsed and processed by guest code (inside xenia > emulator). > ffmpeg replaces a hardware decoder dsp in this case. > You should really use XMA parser, it have nothing to do with RIFF. > _______________________________________________ > 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 2021-09-01 08:26, Paul B Mahol wrote: > On Tue, Aug 31, 2021 at 11:19 PM Joel Linn <jl@conductive.de> wrote: > >> On 2021-08-31 23:13, Paul B Mahol wrote: >> > On Tue, Aug 31, 2021 at 11:10 PM Joel Linn <jl@conductive.de> wrote: >> > >> >> Hello, >> >> >> >> On 2021-08-31 22:53, Paul B Mahol wrote: >> >> > What samples this fixes? >> >> >> >> No specific sample, it depends on the way the decoder is invoked. >> >> >> >> It fixes using the xma1/2 decoder directly on a memory buffer and not >> >> wanting to decode all channels in the stream. >> >> Say the stream has 6 channels and one wants to only decode 2. >> >> Previously this was not possible due to the skip logic of the channel >> >> interleaving. >> >> >> >> This should also make the decoder more robust because in theory it is >> >> possible to have an xma stream which has chunks of 2kB junk data >> >> (packets) which are expected to be skipped due to the skip_packets >> >> numbers. >> >> I haven't seen such streams though. >> >> >> > >> > Am I correct you already use XMA parser? >> >> In my case I was working with raw stream buffers, the XMA format >> (RIFF) >> headers are invisible to me. >> The format is parsed and processed by guest code (inside xenia >> emulator). >> ffmpeg replaces a hardware decoder dsp in this case. >> > > You should really use XMA parser, it have nothing to do with RIFF. Sorry misunderstood you. While xma_parser indeed skips packets correctly, I do not see how that affects buffer parsing in `xma_decode_packet`. I introduced a small bug (variable declaration) in the patch during submission cleanup, will need to submit v2.
diff --git a/libavcodec/wmaprodec.c b/libavcodec/wmaprodec.c index 66271c4037..95e039c14e 100644 --- a/libavcodec/wmaprodec.c +++ b/libavcodec/wmaprodec.c @@ -1814,7 +1814,43 @@ static int xma_decode_packet(AVCodecContext *avctx, void *data, XMADecodeCtx *s = avctx->priv_data; int got_stream_frame_ptr = 0; AVFrame *frame = data; - int i, ret, offset = INT_MAX; + int i, ret, offset, skip_current_packet = INT_MAX; + + /* find the owner stream of the new XMA packet that belongs to on of our streams + * XMA streams find their packets following packet_skips + * there may be other packets in between if we are not responsible for all streams + * (at start there is one packet per stream, then interleave non-linearly). */ + if (s->xma[s->current_stream].packet_done || + s->xma[s->current_stream].packet_loss) { + /* select stream with lowest skip_packets (= uses next packet) */ + if (s->xma[s->current_stream].skip_packets != 0) { + int min[2]; + + min[0] = s->xma[0].skip_packets; + min[1] = i = 0; + + for (i = 1; i < s->num_streams; i++) { + if (s->xma[i].skip_packets < min[0]) { + min[0] = s->xma[i].skip_packets; + min[1] = i; + } + } + + s->current_stream = min[1]; + } + + skip_current_packet = !!s->xma[s->current_stream].skip_packets; + + /* advance all stream packet skip counts */ + for (i = 0; i < s->num_streams; i++) { + s->xma[i].skip_packets = FFMAX(0, s->xma[i].skip_packets - 1); + } + + /* if we are not responsible for every stream, make sure we ignore + * XMA packets not belonging to one of our streams */ + if (skip_current_packet) + return avctx->block_align; + } if (!s->frames[s->current_stream]->data[0]) { s->frames[s->current_stream]->nb_samples = 512; @@ -1846,34 +1882,8 @@ static int xma_decode_packet(AVCodecContext *avctx, void *data, return ret; } - /* find next XMA packet's owner stream, and update. - * XMA streams find their packets following packet_skips - * (at start there is one packet per stream, then interleave non-linearly). */ if (s->xma[s->current_stream].packet_done || s->xma[s->current_stream].packet_loss) { - - /* select stream with 0 skip_packets (= uses next packet) */ - if (s->xma[s->current_stream].skip_packets != 0) { - int min[2]; - - min[0] = s->xma[0].skip_packets; - min[1] = i = 0; - - for (i = 1; i < s->num_streams; i++) { - if (s->xma[i].skip_packets < min[0]) { - min[0] = s->xma[i].skip_packets; - min[1] = i; - } - } - - s->current_stream = min[1]; - } - - /* all other streams skip next packet */ - for (i = 0; i < s->num_streams; i++) { - s->xma[i].skip_packets = FFMAX(0, s->xma[i].skip_packets - 1); - } - /* copy samples from buffer to output if possible */ for (i = 0; i < s->num_streams; i++) { offset = FFMIN(offset, s->offset[i]);