Bug 120665 - [GStreamer] media/track/in-band/ layout tests introduced in r154908 are failing
Summary: [GStreamer] media/track/in-band/ layout tests introduced in r154908 are failing
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Enrique Ocaña
URL:
Keywords: Gtk, LayoutTestFailure
Depends on: 212865
Blocks:
  Show dependency treegraph
 
Reported: 2013-09-04 01:19 PDT by Zan Dobersek
Modified: 2020-06-19 03:58 PDT (History)
20 users (show)

See Also:


Attachments
Patch (1.82 KB, patch)
2013-09-04 11:32 PDT, Brendan Long
no flags Details | Formatted Diff | Diff
Patch (68.87 KB, patch)
2020-06-05 07:20 PDT, Enrique Ocaña
eocanha: review?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2013-09-04 01:19:54 PDT
The media/track/in-band/ layout tests are failing since they were added in r154908.
http://trac.webkit.org/changeset/154908

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=media%2Ftrack%2Fin-band

The VIDEO_TRACK feature seems to be enabled at build-time, but I'm not familiar enough with the feature or tests to investigate at this moment.
Comment 1 Brendan Long 2013-09-04 09:21:42 PDT
It's probably because they need GStreamer 1.1.2 to work correctly, and the test machines don't have that version. Do you want me to just skip these for now?
Comment 2 Zan Dobersek 2013-09-04 11:27:38 PDT
I've already skipped them for the GTK port in r155025.
http://trac.webkit.org/projects/webkit/changeset/155025

They're also failing on EFL though.
http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=media%2Ftrack%2Fin-band
Comment 3 Brendan Long 2013-09-04 11:32:57 PDT
Created attachment 210481 [details]
Patch
Comment 4 WebKit Commit Bot 2013-09-05 06:20:57 PDT
Comment on attachment 210481 [details]
Patch

Clearing flags on attachment: 210481

Committed r155111: <http://trac.webkit.org/changeset/155111>
Comment 5 WebKit Commit Bot 2013-09-05 06:21:01 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Zan Dobersek 2013-09-05 06:31:37 PDT
Reopening since the tests are failing and will continue to fail until we move onto using GStreamer >= 1.1.2.
Comment 7 Brendan Long 2013-09-05 09:10:38 PDT
(In reply to comment #6)
> Reopening since the tests are failing and will continue to fail until we move onto using GStreamer >= 1.1.2.

Did I not skip them correctly, or do you just mean that the fact that they're being skipped means the bug should remain open?
Comment 8 Zan Dobersek 2013-09-05 09:38:18 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > Reopening since the tests are failing and will continue to fail until we move onto using GStreamer >= 1.1.2.
> 
> Did I not skip them correctly, or do you just mean that the fact that they're being skipped means the bug should remain open?

The latter.
Comment 9 Zan Dobersek 2013-09-05 09:41:00 PDT
I reckon that the GStreamer bump will be done at some point in the future, but in a separate bug. Still, until then I'd like to keep one bug entry (i.e. this one) open that notes the reason behind the skipping and what's necessary to get the tests unskipped and passing.
Comment 10 Carlos Alberto Lopez Perez 2014-04-11 09:50:52 PDT
I will change the bug assigned on LayoutTests/platform/gtk/TestExpectations from #103771 to this one for the following tests:

  media/track/track-forced-subtitles-in-band.html
  media/track/track-in-band.html
  media/track/track-in-band-cues-added-once.html
  media/track/track-in-band-style.html
  media/track/track-in-band-legacy-api.html
  media/track/track-in-band-mode.html

I think the above tests are failing for the very same problem discussed on this bug.
Comment 11 Brendan Long 2014-04-11 10:03:28 PDT
(In reply to comment #10)
> I will change the bug assigned on LayoutTests/platform/gtk/TestExpectations from #103771 to this one for the following tests:
> 
>   media/track/track-forced-subtitles-in-band.html
>   media/track/track-in-band.html
>   media/track/track-in-band-cues-added-once.html
>   media/track/track-in-band-style.html
>   media/track/track-in-band-legacy-api.html
>   media/track/track-in-band-mode.html
> 
> I think the above tests are failing for the very same problem discussed on this bug.

Weren't these already skipped? Those tests require MPEG-4 caption support (whatever format that is), and I'm pretty sure GStreamer doesn't support that format.
Comment 12 Carlos Alberto Lopez Perez 2014-04-11 10:25:59 PDT
(In reply to comment #11)
> Weren't these already skipped? Those tests require MPEG-4 caption support (whatever format that is), and I'm pretty sure GStreamer doesn't support that format.

Yes, but they were assigned to bug https://bugs.webkit.org/show_bug.cgi?id=103771 (which seems fixed).

Isn't this the right bug for tracking them? I will open a new bug otherwise
Comment 13 Brendan Long 2014-04-11 10:34:50 PDT
(In reply to comment #12)
> (In reply to comment #11)
> > Weren't these already skipped? Those tests require MPEG-4 caption support (whatever format that is), and I'm pretty sure GStreamer doesn't support that format.
> 
> Yes, but they were assigned to bug https://bugs.webkit.org/show_bug.cgi?id=103771 (which seems fixed).
> 
> Isn't this the right bug for tracking them? I will open a new bug otherwise

I'm not sure. The underlying problem is different. In one case (contents of media/track/in-band/), we can unskip the tests when the build bots have the right version of GStreamer. In the other case (media/track/track-in-band*.html), we need GStreamer to add support for that caption format, then wait for the build bots to get that version.
Comment 14 Carlos Alberto Lopez Perez 2014-04-11 10:57:47 PDT
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > Weren't these already skipped? Those tests require MPEG-4 caption support (whatever format that is), and I'm pretty sure GStreamer doesn't support that format.
> > 
> > Yes, but they were assigned to bug https://bugs.webkit.org/show_bug.cgi?id=103771 (which seems fixed).
> > 
> > Isn't this the right bug for tracking them? I will open a new bug otherwise
> 
> I'm not sure. The underlying problem is different. In one case (contents of media/track/in-band/), we can unskip the tests when the build bots have the right version of GStreamer. In the other case (media/track/track-in-band*.html), we need GStreamer to add support for that caption format, then wait for the build bots to get that version.

Ok. I have opened a new bug for those tests: https://bugs.webkit.org/show_bug.cgi?id=131546
Comment 15 Enrique Ocaña 2020-06-05 07:20:48 PDT
Created attachment 401154 [details]
Patch
Comment 16 Philippe Normand 2020-06-05 07:30:43 PDT
Comment on attachment 401154 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401154&action=review

> Tools/ChangeLog:9
> +        Fixed media/track/in-band tests. Added libkate as a dependency, so gst-plugins-bad detects
> +        it and can build the GstKateDec element.

This won't work on the bots. The SDK doesn't ship libkate currently.
Comment 17 Enrique Ocaña 2020-06-05 07:43:43 PDT
Comment on attachment 401154 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401154&action=review

>> Tools/ChangeLog:9
>> +        it and can build the GstKateDec element.
> 
> This won't work on the bots. The SDK doesn't ship libkate currently.

Then I would need some guidance on how to add the dependency to the SDK. This is a topic totally new for me.

> LayoutTests/media/track/in-band/track-in-band-mpegts-metadata.html:-1
> -<!DOCTYPE html>

Note for reviewers:

I've removed this test because it would never succeed with the current tsdemux implementation. That implementation basically skips unknown streams on the mpegts container ( https://github.com/GStreamer/gst-plugins-bad/blob/1.16/gst/mpegtsdemux/tsdemux.c#L1687 ), and those streams are the ones the test relies on in order to get extra pads (exposed as metadata text tracks).
Comment 18 Enrique Ocaña 2020-06-16 03:19:32 PDT
Comment on attachment 401154 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401154&action=review

>>> Tools/ChangeLog:9
>>> +        it and can build the GstKateDec element.
>> 
>> This won't work on the bots. The SDK doesn't ship libkate currently.
> 
> Then I would need some guidance on how to add the dependency to the SDK. This is a topic totally new for me.

Libkate support has been added in https://trac.webkit.org/changeset/262941/webkit, but still the test don't passed when using flatpak because avdec_mpeg was missing. Later on, https://trac.webkit.org/changeset/263027/webkit added that missing support, so now all the in-band tests (as modified by this patch) are passing here for me with flatpak.
Comment 19 Philippe Normand 2020-06-16 04:30:01 PDT
Comment on attachment 401154 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401154&action=review

> LayoutTests/media/in-band-tracks.js:117
> +        // Sometimes the text tracks take a bit to be found as the media is processed, so we must retry.
> +        if (!inbandTrack1) {
> +            setTimeout(canplaythrough, 100);
> +            return;
> +        }

I'm afraid this could potentially trigger an infinite recursion. Would it make sense to perform a fixed amount and fail the test otherwise?
Comment 20 Eric Carlson 2020-06-16 09:33:47 PDT
Comment on attachment 401154 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=401154&action=review

>> LayoutTests/media/in-band-tracks.js:117
>> +        }
> 
> I'm afraid this could potentially trigger an infinite recursion. Would it make sense to perform a fixed amount and fail the test otherwise?

I agree that checking for a fixed amount of time before failing the test would be better, as all state is lost when WKTR kills a test for taking too long.
Comment 21 Enrique Ocaña 2020-06-19 03:58:12 PDT
All the tracks are found now at the fist try, so apparently there's no need to retry (setTimeout) anymore. I don't know if this started to happen after having migrated to the flatpak-based development environment or if it was just chance.

On the other hand, now I'm experiencing some flakiness in some of the tests that are requiring some debugging. The next version of the patch will take more than expected.