515148 - Decode spins in oggplay_step_decoding loading Ogg Flac

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Enrique]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; es-ES; rv:1.9.1) Gecko/20090624 Firefox/3.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; es-ES; rv:1.9.1) Gecko/20090624 Firefox/3.5

 While making a first test with the <audio> tag the browser went to an infinite loop wasting 70% of CPU cicles.

Reproducible: Always

Steps to Reproduce:
1. Load a draft html page (attached file) with an audio tag.
2.
3.
Actual Results:  
(See summary)

Expected Results:  
Listening an ogg file

(see attached files with sample html file, ogg file, strace & lsof dumps)

Comment 1

[Enrique]

Attachment: tar.gz file with sample html, ogg, lsof&strace files

Comment 2

[Carlos Alén Silva]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3

Confirmed on Windows XP SP 3. CPU usage was 99% - 100%. Luckily, everything goes back to normal when closing the tab running sample.html.

Comment 3

[Matthew Gregan [:kinetik]]

Thanks for the bug report!  Your file contains audio data compressed with the Flac codec, which is not supported by Firefox.  We should indicate that we're unable to play this format by firing an error event at the element and (if controls are enabled) displaying a visual indication of an error.

The decode thread is spinning in oggplay_step_decoding, calling via DecodeFrame after LoadOggHeaders.  mVideoTrack == mAudioTrack == -1 in this case, so there's nothing to decode.  oggplay should return an error rather than spinning, but we shouldn't be trying to decode frames for this file when we have not activated any tracks.

It turns out that bug 481057 would fix this, because it fires a decode error in LoadOggHeaders when mVideoTrack == mAudioTrack == -1, which will cause us to shut down the decoder.

Comment 4

[Viktor Gal]

The backed out patch would fix this problem (bug 512328).

Comment 5

[Matthew Gregan [:kinetik]]

Viktor, I tested this again with the liboggplay update in the tree (and my patch for bug 481057 backed out, so that we still call oggplay_step_decoding despite having zero activated tracks) and it still ends up stuck in an infinite loop:

In oggplay_step_decoding, we take this path:

    if (me->active_tracks == 0) {
...
          remaining++;
...
    for (i = 0; i < me->num_tracks; i++) {
      if (me->decode_data[i]->active == 0)
        continue;
...
    if (!need_data) {
      break;
...
  num_records = oggplay_callback_info_prepare (me, &info);
  if (info != NULL) {
  } else {
    r = 0;
  }
...
  if (info == NULL) {
    goto read_more_data;

Comment 6

[Matthew Gregan [:kinetik]]

Marking this as resolved since bug 481057 landed.  I've filed the oggplay infinite loop bug upstream as Annodex ticket #496.