496063 - Infinite loop entered when shutting down a video during a seek operation

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[cajbir (:cajbir)]

When shutting down during a mochitest run the browser entered an infinite loop. Debugging showed the the decoder thread was spinning in the do/while in liboggz file oggz_seek.c lines 771-774:

  do {
    offset_at = oggz_get_prev_start_page (oggz, og, &granule_at, &serialno);
    unit_at = oggz_get_unit (oggz, serialno, granule_at);
  } while (unit_at > unit_target);

oggz_get_prev_start_page returns -1 (via calling out media stream code which bails out with the error when noticing we're closed/shutting down). The result stored in offset_at is not checked and the loop never exits.

The oggz_get_unit call does no file i/o and operates on the current granule position, which hasn't changed due to the previous error.

This may be the cause of bug 471085.

Comment 1

[cajbir (:cajbir)]

Attachment: Fix

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 381221 [details] [diff] [review]
Fix


>@@ -476,16 +476,18 @@ oggz_scan_for_page (OGGZ * oggz, ogg_page * og, ogg_int64_t unit_target,
>       do {
>         offset_at = oggz_get_prev_start_page(oggz, og, &granule_at, &serialno);
>+	if (offset_at < 0)
>+	  break;
>         unit_at = oggz_get_unit(oggz, serialno, granule_at);
>       } while (unit_at > unit_target);
>       return offset_at;

Nit: Use spaces not tabs.

Otherwise looks good!

Comment 3

[cajbir (:cajbir)]

I think we should have this in 1.9.1. It fixes an infinite loop which causes 100% cpu usage in a background thread and can cause shutdown hangs.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 381221 [details] [diff] [review]
Fix

a191=beltzner

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/mozilla-central/rev/ad31c4f61407

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c1816ae0a4a5

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Chris, do you have a testcase for this bug? Or will all video files work to reproduce this issue?

Comment 8

[cajbir (:cajbir)]

No testcase, sorry. The shutdown has to occur while the decoder is at a specific point in the code during a seek operation. I was lucky to hit it while doing other things and had a debugger handy to track it down.

Comment 9

[Conrad Parker]

Applied in upstream liboggz commit 412768eb6b05ca57664c70d2728e491b3c31590f