Video without autoplay doesn't show up #31666

mrego · 2024-03-14T11:52:28Z

The following example:

<video>
  <source src="https://interactive-examples.mdn.mozilla.net/media/cc0-videos/flower.mp4" type="video/mp4">
  <p>Video not supported</video>
</video>

Shows Video not supported.

However simply adding autoplay makes the video to show and play properly:

<video autoplay>
  <source src="https://interactive-examples.mdn.mozilla.net/media/cc0-videos/flower.mp4" type="video/mp4">
  <p>Video not supported</video>
</video>

Maybe this is a known/duplicated issue, sorry if that's the case.

The text was updated successfully, but these errors were encountered:

aditi5926 · 2024-03-15T13:12:14Z

So basically you want the reason why is this happening right? No code is required right?

jdm · 2024-03-15T14:52:38Z

This issue can only be closed when videos that do not have the autoplay attribute are rendered correctly. That will require code changes.

RustAndMetal · 2024-03-19T23:39:30Z

I just spent some time looking into this, for anyone else interested in this issue, here's some things that I thought might be of note to someone working on this issue (not exhaustive but maybe it is helpful to someone):

The section of the spec that is violated here is this:

When the video element is paused, the current playback position is the first frame of video, and the element's show poster flag is set
The video element represents its poster frame, if any, or else the first frame of the video.

https://html.spec.whatwg.org/multipage/media.html#the-video-element:the-video-element-9

Poster frames already partially work, so there does appear to be some code path that displays "something else" when the above condition is met:

<video poster="https://placehold.co/500x500.jpg">
    <source src="https://interactive-examples.mdn.mozilla.net/media/cc0-videos/flower.mp4" type="video/mp4">
    <p>Video not supported</p>
</video>

Note that even then, before the video is loaded, it is not displayed.

The issue isn't "really" with autoplay but just with all paused videos at the first frame. Using JavaScript to play a video without an autoplay tag makes it show up once the first frame is rendered:

<video>
    <source src="https://interactive-examples.mdn.mozilla.net/media/cc0-videos/flower.mp4" type="video/mp4">
    <p>Video not supported</p>
</video>
<script>
    setTimeout(() => {
        document.querySelector('video').play();
    }, 1000);
</script>

This might actually be an issue in the media crate or might at least require code changes there too, to allow us to properly show the first frame. Though this is just speculation, maybe someone with more experience in this part of the codebase can shed some light on this.

eerii · 2024-03-29T00:30:51Z

@RustAndMetal thank you so much for the insight! I did some testing and it seems that #31746 addresses some of the issues. However, it still doesn't show the first frame of the video as the placeholder. Here's a comparison:

Current behaviour:

With #31746:

Testing with:

<!DOCTYPE html>
<body>
    <video style="border: 1px solid red">
    <source src="https://interactive-examples.mdn.mozilla.net/media/cc0-videos/flower.mp4" type="video/mp4"/>
    <p>Video not supported</p>
  </video>

  <video autoplay style="border: 1px solid blue">
    <source src="https://interactive-examples.mdn.mozilla.net/media/cc0-videos/flower.mp4" type="video/mp4"/>
    <p>Video not supported</p>
  </video>

  <video poster="https://placehold.co/500x500.jpg" style="border: 1px solid yellow">
    <source src="https://interactive-examples.mdn.mozilla.net/media/cc0-videos/flower.mp4" type="video/mp4"/>
    <p>Video not supported</p>
  </video>

  <video width="400" style="border: 1px solid green">
    <source src="https://interactive-examples.mdn.mozilla.net/media/cc0-videos/flower.mp4" type="video/mp4"/>
    <p>Video not supported</p>
  </video>
</body>

The videos are definitely loading and the size is set correctly (the red rectangle is the same as the blue one, the other two are different because either the poster sets the size or the default width was modified). I'll take a look at the code tomorrow to see if I can get the first frame to render by default.

eerii · 2024-03-29T15:31:01Z

Update: I have been looking into this and going into servo-media and gstreamer code. One way this could be solved is by adding a function to servo-media's Player that requests a frame to be rendered (without starting play the video), something like request_frame. We need a way to render one single video frame without playing the video, and as far as I could see servo-media doesn't yet have that interface.

Inside this function we could call gst_player_get_video_snapshot, which returns a Sample with the current frame of the video. With the Sample we can then call the renderer's get_frame_from_sample, which gives us a VideoFrame. Finally, this can be used to call VideoFrameRenderer::render(), which is implemented in htmlmediaelement.rs by MediaFrameRenderer. This should update the current frame so it shows on screen just like when a video is paused.

fn request_frame(&self) -> Result<(), PlayerError> {
    let inner = self.inner.borrow();
    let mut inner = inner.as_ref().unwrap().lock().unwrap();
    let Some(sample) = inner.player.video_snapshot(...) else {
        return Err(...);
    };
    let Ok(frame) = self.render.lock().unwrap().get_frame_from_sample(sample) else {
        return Err(...);
    };
    if let Some(video_renderer) = &self.video_renderer {
        video_renderer.lock().unwrap().render(frame);
    }
    Ok(())
}

The new function could be called somewhere around here, when ReadyState::HaveEnoughData is detected and self.autoplaying is disabled. I have a very messy implementation of all of this, but I'm still trying to solve a few issues. The main one is that I am getting

GLib-GObject-CRITICAL **: 16:17:12.052: g_object_get_is_valid_property: object class 'GstPlayBin3' has no property named 'n-video'

when calling gst_player_get_video_snapshot, which looking at gstreamer's gst_play_get_video_snapshot outside of the rust wrapper leads me to believe that I am calling it when the video is not yet initialized, so more testing will need to be done.

g_object_get (self->playbin, "n-video", &video_tracks, NULL); // This crashes
if (video_tracks == 0) {
    GST_DEBUG_OBJECT (self, "total video track num is 0");
    return NULL;
}

I'll try to diagnose the 'n-video' error and get this working, but if anyone that is familiar with gstreamer has any pointers they are greatly appreciated!

jdm · 2024-03-29T15:37:40Z

I can't provide any gstreamer assistance, but that all sounds pretty reasonable to me!

eerii · 2024-04-03T00:20:40Z

I have a working proof of concept for this!

The critical error was coming directly from a GStreamer regression on the latest version, I opened a pr there (https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/6516). I also had to find another place from where to call request_frame in htmlmediaelement, since it needs the video buffer to exist before extracting a sample, and using PlayerEvent::StateChanged with PlaybackState::Paused seems to do the trick.

The implementation still needs a bit of cleanup, as well as handling some details like asking video_snapshot for the correct size, checking if a poster already exists and reviewing the spec and test expectations so it is fully compatible.

The change will require two prs, one in servo/media that adds the request_frame player trait function, and one here with the changes to htmlmediaelement. I'll try to have the first one ready tomorrow.

ceyusa · 2024-04-03T11:19:38Z

Hi @eerii

Thanks for your hard work. Your proposal of a video snapshot is interesting and it looks to solve the issue. But I wonder how required is too add new code for that.

As you already found out, in a GStreamer pipeline, before passing to PLAYING state, it has to change first to PAUSED state, and in to reach PAUSED state, all sinks have to receive a buffer, this operation is known as preroll. So, as far as I remember, when the GStreamer's pipeline pass to PAUSED, the ReadyState::HaveEnoughData is emitted. Then, there should be already a video frame to draw, the first one received at preroll, and that should be drawn.

All I'm saying that perhaps it's a bug to fix rather than a workaround to add. Not sure, though.

eerii · 2024-04-03T13:42:17Z

Thank you so much for all the information, the page on preroll was very useful.

So, as far as I remember, when the GStreamer's pipeline pass to PAUSED, the ReadyState::HaveEnoughData is emitted. Then, there should be already a video frame to draw, the first one received at preroll, and that should be drawn.

This makes a lot of sense, I observed thatgst_play_get_video_snapshot only works when there is a buffer loaded, for example, after switching to the PAUSED state. This makes sense because internally it is emitting a convert-sample event, and according to this it returns NULL when there is no current buffer:

Returns (GstSample *) – a GstSample of the current video frame converted to caps. The caps on the sample will describe the final layout of the buffer data. NULL is returned when no current buffer can be retrieved or when the conversion failed.

Additionally, if no caps is passed to the convert-sample event it just returns the sample property and no conversion is done, so there shouldn't be too much overhead to call this.

All I'm saying that perhaps it's a bug to fix rather than a workaround to add. Not sure, though.

I'll look closely into the code to see if it's supposed to be rendered somewhere already. As far as I could tell before, the buffer was loaded but no calls to render were made, but I'll review the comments and todos just in case. There needs to be a place where we render this initial frame, maybe it can be done directly on servo/media when the video first enters the PAUSED state, I'll look into it. Thank you a lot!

eerii · 2024-04-03T16:00:19Z

You were right! In gstreamer/player.rs there is already a callback for rendering new samples, which also defines a callback for when a preroll is available, but right now it doesn't do anything. This is probably the best place to call the renderer with the prerolled buffer, just like when a new sample is generated. That was a great catch!

eerii · 2024-04-03T18:39:35Z

I created a new pr in servo/media which renders the first frame on the preroll callback. There are two issues but they need to be resolved in this repo:

Intrinsic sizing has to apply to the rendered frame, but this is better suited as an addition to [WIP]: Improve how intrinsic sizes work for videos #31746.
Not yet compliant with the spec, because it shows the first frame over the poster if it exists (The video element represents its poster frame, if any, or else the first frame of the video).

mrego added the A-content/media label Mar 14, 2024

eerii mentioned this issue Apr 3, 2024

Render first video frame when preroll is available servo/media#419

Merged

mrobinson closed this as completed in servo/media#419 Apr 4, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Video without autoplay doesn't show up #31666

Video without autoplay doesn't show up #31666

mrego commented Mar 14, 2024

aditi5926 commented Mar 15, 2024

jdm commented Mar 15, 2024

RustAndMetal commented Mar 19, 2024

eerii commented Mar 29, 2024 •

edited

eerii commented Mar 29, 2024 •

edited

jdm commented Mar 29, 2024

eerii commented Apr 3, 2024

ceyusa commented Apr 3, 2024 •

edited

eerii commented Apr 3, 2024

eerii commented Apr 3, 2024

eerii commented Apr 3, 2024

Video without autoplay doesn't show up #31666

Video without autoplay doesn't show up #31666

Comments

mrego commented Mar 14, 2024

aditi5926 commented Mar 15, 2024

jdm commented Mar 15, 2024

RustAndMetal commented Mar 19, 2024

eerii commented Mar 29, 2024 • edited

eerii commented Mar 29, 2024 • edited

jdm commented Mar 29, 2024

eerii commented Apr 3, 2024

ceyusa commented Apr 3, 2024 • edited

eerii commented Apr 3, 2024

eerii commented Apr 3, 2024

eerii commented Apr 3, 2024

eerii commented Mar 29, 2024 •

edited

eerii commented Mar 29, 2024 •

edited

ceyusa commented Apr 3, 2024 •

edited