[FFmpeg-devel] [PATCH 1/2] wmavoice: truncate spillover_nbits if too large

[Andreas Cadhalpun]

On 01.01.2017 23:27, Ronald S. Bultje wrote:
> On Sun, Jan 1, 2017 at 5:18 PM, Andreas Cadhalpun <andreas.cadhalpun at googlemail.com <mailto:andreas.cadhalpun at googlemail.com>> wrote:
> 
>     This fixes triggering the av_assert0(ret <= tmp.size).
> 
>     The problem was reintroduced by commit
>     7b27dd5c16de785297ce4de4b88afa0b6685f61d and originally fixed by
>     2a4700a4f03280fa8ba4fc0f8a9987bb550f0d1e.
> 
>     Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com <mailto:Andreas.Cadhalpun at googlemail.com>>
>     ---
>      libavcodec/wmavoice.c | 5 +++++
>      1 file changed, 5 insertions(+)
> 
>     diff --git a/libavcodec/wmavoice.c b/libavcodec/wmavoice.c
>     index cd5958c7bc..1bfad46b2e 100644
>     --- a/libavcodec/wmavoice.c
>     +++ b/libavcodec/wmavoice.c
>     @@ -1923,6 +1923,11 @@ static int wmavoice_decode_packet(AVCodecContext *ctx, void *data,
>               * continuing to parse new superframes in the current packet. */
>              if (s->sframe_cache_size > 0) {
>                  int cnt = get_bits_count(gb);
>     +            if (cnt + s->spillover_nbits > avpkt->size * 8) {
>     +                av_log(ctx, AV_LOG_WARNING, "Number of spillover bits %d larger than remaining packet size %d, truncating.\n",
>     +                       s->spillover_nbits, avpkt->size * 8 - cnt);
>     +                s->spillover_nbits = avpkt->size * 8 - cnt;
>     +            }
> 
> 
> This litters stderr uselessly. It doesn't help a user or developer accomplish
> anything. Let me explain, because this is probably confusing.
> 
> If a library integrator (e.g., the developer of gst-ffmpeg) is playing with
> wmavoice and forgets to set block_align, the codec should error out. Ideally,
> it does this saying block_align is zero, so the developer knows what to do.
> But this is not strictly required and we typically don't do this.
> 
> However, the above can only happen during corrupt streams (or fuzzing).
> The user cannot fix this. So any detailed error is useless.

I don't think it's useless, as it tells the user that there is a problem with
the file. He might just test if ffmpeg can decode it to see if it is corrupt.

> Therefore, we should not display any detailed error (or warning) message.

So what would you do instead?

Best regards,
Andreas