Intermittent Youtube error "An error occurred. Please try again later"

RESOLVED FIXED in Firefox 45

Status

()

RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: jya, Assigned: jya)

Tracking

(Blocks: 1 bug)

Trunk
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox44 wontfix, firefox45+ fixed, firefox46 fixed, firefox47 fixed)

Details

Attachments

(6 attachments)

(Assignee)

Description

3 years ago
This has happened a few times over the week-end while trying to reproduce bug 1246358.

This is on a "MacBook Pro (Retina, 13-inch, Early 2015)"

Unfortunately, I couldn't reproduce it after attempting to increase the log level which made it impossible to reproduce.
This typically happened immediately after starting. Either it would always fail until I restarted Nightly, or it would work fine.

This doesn't appear to be related to the media stack: there's no decoding error, demuxing error nor MSE error.

All I could see in this log was an init error related to graphics.

Unfortunately the logs appear to have disappeared (had to reboot the laptop).
Will post them again when I reproduce the problem or if I manage to exhume the logs.

I wonder if this is the same as bug 1235967.
(Assignee)

Comment 1

3 years ago
Created attachment 8716810 [details]
erroryoutube.log

The error is:
[74954] WARNING: RasterImage::Init failed: file /Users/jyavenard/Work/Mozilla/mozilla-central/image/ImageFactory.cpp, line 109
[74954] WARNING: RasterImage::Init failed: file /Users/jyavenard/Work/Mozilla/mozilla-central/image/ImageFactory.cpp, line 109

This is the only thing abnormal I could find in the logs. When playback started, this warning didn't appear.
Flags: needinfo?(nical.bugzilla)
Flags: needinfo?(matt.woodrow)
(Assignee)

Comment 2

3 years ago
Created attachment 8716823 [details]
erroryoutube2.log

This times it happened with more logging, and much earlier; making for a rather short log:
We have the same two:
[5121] WARNING: RasterImage::Init failed: file /Users/jyavenard/Work/Mozilla/mozilla-central/image/ImageFactory.cpp, line 109
[5121] WARNING: RasterImage::Init failed: file /Users/jyavenard/Work/Mozilla/mozilla-central/image/ImageFactory.cpp, line 109

nightly was started with NSPR_LOG_MODULES=PlatformDecoderModule:5,MediaFormatReader:5,MediaDecoder:5

JW, can you see what is output the error? i see no error in those logs. What gives Youtube its cues that something wrong occurred?
Flags: needinfo?(jwwang)
(Assignee)

Comment 3

3 years ago
preliminary STR:

Somehow I managed to reproduce more often with this URL:
https://www.youtube.com/watch?v=JLf9q36UsBk&feature=youtu.be&t=254

It's likely unrelated as typically it happens when I restore from an old session.

But directly opening https://www.youtube.com/watch?v=JLf9q36UsBk&feature=youtu.be&t=254 as often produce the problem. (note that you need bug 1246358 applied, otherwise you'll hit another bug first)
(Assignee)

Comment 4

3 years ago
Created attachment 8716829 [details]
erroryoutube3.log

Another log with just MediaDecoder:5,MediaSource:5

shows that there are no MSE error in there...
(Assignee)

Comment 5

3 years ago
This is youtube debug info.
{"ns":"yt","el":"detailpage","cpn":"QXsGf2vpomxg448r","docid":"JLf9q36UsBk","ver":2,"referrer":null,"cmt":"256.581","plid":"AAUrNyCfP2pfK338","ei":"nd63VqjdOsvH4AKwgo0g","fmt":"160","fs":"0","rt":"1658.822","of":"Y2jYQoPFrCFKbJ5Geg8v4A","adformat":null,"content_v":null,"euri":"","subscribed":null,"lact":38,"live":null,"cl":"113861800","mos":0,"osid":null,"state":"80","vm":"CAMQAQ","volume":100,"c":"WEB","cver":"html5","cplayer":"UNIPLAYER","cbr":"Chrome","cbrver":"46.0.2490.80","cos":"Windows","cosver":"6.1","hl":"en_US","cr":"AU","len":"256.581","fexp":"9406983,9416126,9420452,9422596,9423661,9423662","feature":"youtu.be","afmt":"250","vct":"0.000","vd":"NaN","vpl":"","vbu":"","vpa":true,"vsk":false,"ven":false,"vpr":1,"vrs":0,"vns":0,"vec":null,"vvol":0.6294337190892868,"debug_error":{"errorCode":"fmt.unplayable","errorDetail":"invalidState.1","message":"An error occurred. Please try again later.","messageKey":"YTP_ERROR_GENERIC_WITHOUT_LINK"},"debug_videoId":"JLf9q36UsBk","gpu":"Intel(R)_Iris(TM)_Graphics_6100","cgr":true,"debug_playbackQuality":"tiny","debug_date":"Mon Feb 08 2016 13:34:01 GMT+1100 (AEDT)"}

So at least this appear different to bug 1235967
(Assignee)

Comment 6

3 years ago
Ok found it.

This is a MSE error due to a race.

The audio track is shorter than the video track.
So the media element enter ended state due to the audio track finishing early. It may happen that YouTube is still trying to append new data.
https://dxr.mozilla.org/mozilla-central/source/dom/media/mediasource/SourceBuffer.cpp#544

The comment is good, but the test to check if the error attribute is not null is wrong. Being in Ended mode doesn't mean an error always occurred.
Blocks: 778617
Flags: needinfo?(nical.bugzilla)
Flags: needinfo?(matt.woodrow)
Flags: needinfo?(jwwang)
(Assignee)

Comment 7

3 years ago
Created attachment 8716843 [details] [diff] [review]
P1. Add MediaDecoderOwner::HasError method.
Attachment #8716843 - Flags: review?(gsquelart)
(Assignee)

Comment 8

3 years ago
Created attachment 8716844 [details] [diff] [review]
P2. Add MediaDecoder::OwnerHasError method.
Attachment #8716844 - Flags: review?(gsquelart)
(Assignee)

Comment 9

3 years ago
Created attachment 8716845 [details] [diff] [review]
[MSE] P3. Only error during Prepare Append algorithm if the media element is in error.
Attachment #8716845 - Flags: review?(gsquelart)
Attachment #8716843 - Flags: review?(gsquelart) → review+
Comment on attachment 8716844 [details] [diff] [review]
P2. Add MediaDecoder::OwnerHasError method.

Review of attachment 8716844 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nits:

::: dom/media/MediaDecoder.cpp
@@ +1046,5 @@
> +MediaDecoder::OwnerHasError() const
> +{
> +  MOZ_ASSERT(NS_IsMainThread());
> +
> +  return mShuttingDown || mOwner->HasError();

Empty line between statement, unlike the methods above and below. Consistency please!

::: dom/media/MediaDecoder.h
@@ +236,5 @@
>  
> +  // Return true if the decoder is in an error state or is shutting down.
> +  // A decoder is in an error state if the MediaDecoderOwner is in an error
> +  // state.
> +  bool OwnerHasError() const;

The function name doesn't quite match the meaning, it focuses on the owner error state at the expense of the shutdown state.
How about "HasErrorOrShutdown()", in the style of the method above?
Attachment #8716844 - Flags: review?(gsquelart) → review+
Comment on attachment 8716845 [details] [diff] [review]
[MSE] P3. Only error during Prepare Append algorithm if the media element is in error.

Review of attachment 8716845 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with nit (related to a P2 nit) :

::: dom/media/mediasource/SourceBuffer.cpp
@@ +542,5 @@
>    // If the HTMLMediaElement.error attribute is not null, then throw an
>    // InvalidStateError exception and abort these steps.
>    if (!mMediaSource->GetDecoder() ||
> +      mMediaSource->GetDecoder()->OwnerHasError()) {
> +    MSE_DEBUG("HTMLMediaElement.error is not null");

OwnerHasError() could return true if the decoder is shutting down, but the debugging comment doesn't say anything about that.
Attachment #8716845 - Flags: review?(gsquelart) → review+
(In reply to Gerald Squelart [:gerald] from comment #10)
> Comment on attachment 8716844 [details] [diff] [review]
> P2. Add MediaDecoder::OwnerHasError method.
> 
> Review of attachment 8716844 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+ with nits:
> 
> ::: dom/media/MediaDecoder.cpp
> @@ +1046,5 @@
> > +MediaDecoder::OwnerHasError() const
> > +{
> > +  MOZ_ASSERT(NS_IsMainThread());
> > +
> > +  return mShuttingDown || mOwner->HasError();
> 
> Empty line between statement, unlike the methods above and below.
> Consistency please!
> 
> ::: dom/media/MediaDecoder.h
> @@ +236,5 @@
> >  
> > +  // Return true if the decoder is in an error state or is shutting down.
> > +  // A decoder is in an error state if the MediaDecoderOwner is in an error
> > +  // state.
> > +  bool OwnerHasError() const;
> 
> The function name doesn't quite match the meaning, it focuses on the owner
> error state at the expense of the shutdown state.
> How about "HasErrorOrShutdown()", in the style of the method above?

After discussion with Jean-Yves, I see that the "shutdown" part is only needed to handle special cases where no owner is known.
So instead of changing the method name, I would suggest clarifying the comment, e.g.:
// Return true if the MediaDecoderOwner is in an error state.
// Note: If the decoder is shutting down, there is no owner and this is considered an error.

Knowing that, you may ignore my related nit in part 3.

Comment 14

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/de6c0f937e9f
https://hg.mozilla.org/mozilla-central/rev/770d1bb1ca0c
https://hg.mozilla.org/mozilla-central/rev/02c902f19190
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
(Assignee)

Updated

3 years ago
status-firefox44: --- → wontfix
status-firefox45: --- → affected
status-firefox46: --- → affected
(Assignee)

Updated

3 years ago
Assignee: nobody → jyavenard
(Assignee)

Comment 15

3 years ago
[Tracking Requested - why for this release]: Can cause YouTube stall
tracking-firefox45: --- → ?
(Assignee)

Comment 16

3 years ago
correction above: Can cause YouTube errors.
Jean-Yves, could you fill the uplift request to aurora & beta? merci
tracking-firefox45: ? → +
Flags: needinfo?(jyavenard)
(Assignee)

Comment 18

3 years ago
Comment on attachment 8716843 [details] [diff] [review]
P1. Add MediaDecoderOwner::HasError method.

Approval Request Comment
[Feature/regressing bug #]: 1246521
[User impact if declined]: YouTube will stop with a decode error; wrong MSE spec implementation emitting an error when it shouldn't
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: Low. We're doing the right thing
[String/UUID change made/needed]: None
Flags: needinfo?(jyavenard)
Attachment #8716843 - Flags: approval-mozilla-beta?
Attachment #8716843 - Flags: approval-mozilla-aurora?
Comment on attachment 8716843 [details] [diff] [review]
P1. Add MediaDecoderOwner::HasError method.

Improve youtube support, taking it.
Should be in 45 beta 5.
Attachment #8716843 - Flags: approval-mozilla-beta?
Attachment #8716843 - Flags: approval-mozilla-beta+
Attachment #8716843 - Flags: approval-mozilla-aurora?
Attachment #8716843 - Flags: approval-mozilla-aurora+
has problems to apply to beta:

grafting 328332:2df14892dbc7 "Bug 1246521: P1. Add MediaDecoderOwner::HasError method. r=gerald, a=sylvestre"
merging dom/html/HTMLMediaElement.cpp
merging dom/html/HTMLMediaElement.h
merging dom/media/MediaDecoderOwner.h
merging dom/media/gtest/MockMediaDecoderOwner.h
warning: conflicts while merging dom/media/gtest/MockMediaDecoderOwner.h! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use hg resolve and hg graft --continue)
Flags: needinfo?(jyavenard)
You need to log in before you can comment on or make changes to this bug.