Closed Bug 969290 Opened 10 years ago Closed 8 years ago

[Media Recorder][Desktop] Recorder video clips result can't replay /seekable on firefox

Categories

(Core :: Audio/Video: Recording, defect, P2)

Product:
Core
Component:
Audio/Video: Recording
Platform:
x86_64
Linux
Type:
defect

Tracking

()

Status:
RESOLVED DUPLICATE of bug 657791

People

(Reporter: rlin, Assigned: bryce)

References

Details

Attachments

(6 files, 1 obsolete file)

data(1).webm
1.06 MB, video/webm
Details
data(3).webm, with bechen fix
2.84 MB, video/webm
Details
element lengths in data(3).webm
15.50 KB, text/plain
Details
script generating the element lengths list
2.07 KB, text/x-python
Details
Test case for recording video, to check for non seekable video file.
2.42 KB, text/plain
Details
Add a single CueClusterPosition to make recorded WebM video seekable.
8.90 KB, patch
kinetik
: review-
Details | Diff | Splinter Review
Attached video data(1).webm — 
On 20140207 nightly build, 
1. use media recorder to record video content. 
2. play the blob in video tag
3. can play, but can't replay /seekable
Attachment is the result of blob.
Hi Matthew,
I test on ubuntu movie player and it can seekable /replay after one run play.
Any suggestion for that?
Component: Video/Audio → Video/Audio: Recording
There seems to be a bug in the WebM writer.  In the WebM file attached by Randy Lin above, the first cluster starts at 4578 bytes (0x11e2) and has 0x7725 (30501 bytes) in the length field.  According to this length, the cluster should end at 35091 bytes (0x8913), which is where the first SimpleBlock element ends, and a second SimpleBlock element begins.  Presumably this is not where the cluster is intended to end, and the length being written is wrong.

I've been seeing the same issue with video recorded with MediaRecorder in the current nightly build of Firefox (2014-04-21).
Depends on: 994557
I believe the cluster header of the attachment is wrong because I'm fixing it at bug 994557.
This bug would try to set the duration when js side isn't set timeslice or don't call the requestdata, so media recorder can modify the bitstream on temp file.
I test to set the duration as test value and it can make the output video clips seekable.
Jim, could you exam the new result again? :)
I'm still seeing the cluster header problem in data(3).webm.

0x11e7: Cluster, length = 0x41ee (indicating end at 0x53e1)
0x11f3: Timecode
0x11f6: SimpleBlock
0x53e1: SimpleBlock
At least in data(3).webm, it looks like the clusters alternate between having an incorrect length and having an unspecified length.
Great! Hi  Benjamin, could you help to fix this?
Depends on: 999364
I have not tested in Linux, but I am getting this issue on both Windows and Mac versions of Firefox, both 41 and 43a. I have created a test case.
I've tested it on Linux, Mac and Windows on 42 and 43.0b2 and have always got the same issue, the video is not seekable.
Who's handling the webm encoder nowadays?
Flags: needinfo?(ajones)
Rank: 21
Priority: -- → P2
Hi there, it's bad this is not fixed. It vanishes the API itself, I guess 99% of use cases requires to playback and seek video after recording. I don't believe this API should be considered shipped if there is such a problem. I'm writing this just to underline the importance of fixing it. Thanks.
I believe this bug is a duplicate of bug 982573: lack Cues element.

I had tried to write Cues to the end of the file but Firefox would ignore Cues after clusters.
So my WIP write a single CuesClusterPosition that points to the first cluster.
A single CuesClusterPosition means little, but it makes the recorded file slow-seekable (decode from 0s until target time).
test jsfiddle: http://jsfiddle.net/fantasy4z/o8sa3xj3/3/

Anyway I have no idea if this is a acceptable practice so no request for review.
Interestingly, they are working on this in Chrome and files created are not seekable either. Version 49.0.2588.0 canary (64-bit)

I have a simple web app I have been working on too. https://jsfiddle.net/mattybalaam/2gkvddme/22/
Comment on attachment 8697516 [details] [diff] [review]
WIP: add a single CueClusterPosition to make recorded WebM video seekable.

Hello Benjamin,
could you help to have some feedback for my WIP? Thanks.
Attachment #8697516 - Flags: feedback?(bechen)
Comment on attachment 8697516 [details] [diff] [review]
WIP: add a single CueClusterPosition to make recorded WebM video seekable.

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

It is great that the patch fix the replay issue.

::: dom/media/webm/EbmlComposer.cpp
@@ +48,5 @@
> +
> +      uint64_t cuesPosition = ebml.offset - firstElementOffset;
> +      uint64_t aCueClusterPositionOffset;
> +      WriteCues(ebml, aCueClusterPositionOffset);
> +      WriteSeekHead(ebml, cuesPosition);

I'm not fully understand about the relation between seekhead and cues. Could you give me a brief explanation? And why we write the cues before seekhead.

@@ +54,5 @@
> +      uint64_t currentOffset = ebml.offset;
> +      ebml.offset = aCueClusterPositionOffset;
> +      EBML_DEBUG("Write CueClusterPosition: %x",
> +                 currentOffset - firstElementOffset);
> +      Ebml_SerializeUnsigned64(&ebml, CueClusterPosition, 

nit: space.
Attachment #8697516 - Flags: feedback?(bechen) → feedback+
(In reply to Benjamin Chen [:bechen] from comment #19)
> Comment on attachment 8697516 [details] [diff] [review]
> WIP: add a single CueClusterPosition to make recorded WebM video seekable.
> 
> Review of attachment 8697516 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> It is great that the patch fix the replay issue.
> 
> ::: dom/media/webm/EbmlComposer.cpp
> @@ +48,5 @@
> > +
> > +      uint64_t cuesPosition = ebml.offset - firstElementOffset;
> > +      uint64_t aCueClusterPositionOffset;
> > +      WriteCues(ebml, aCueClusterPositionOffset);
> > +      WriteSeekHead(ebml, cuesPosition);
> 
> I'm not fully understand about the relation between seekhead and cues. Could
> you give me a brief explanation? And why we write the cues before seekhead.
> 

SeekHead contains the position of other level 1 elements,
and WebM container guide[1] indicates "WebM SHOULD contain the SeekHead element."
to allow the client to know if the file contains a Cues element.

We write Cues before SeekHead just because we could know the position of Cues when writing SeekHead.
If we want to write Cues after SeekHead,
SeekPosition of Cues should be rewritten after Cues element is finished,
just like how we deal with CueClusterPosition of the first cluster.

[1]: http://www.webmproject.org/docs/container/

> @@ +54,5 @@
> > +      uint64_t currentOffset = ebml.offset;
> > +      ebml.offset = aCueClusterPositionOffset;
> > +      EBML_DEBUG("Write CueClusterPosition: %x",
> > +                 currentOffset - firstElementOffset);
> > +      Ebml_SerializeUnsigned64(&ebml, CueClusterPosition, 
> 
> nit: space.

Thanks for your feedback!
Comment on attachment 8698329 [details] [diff] [review]
Add a single CueClusterPosition to make recorded WebM video seekable.

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

Hi Ralph: please help to review, thanks.
Attachment #8698329 - Flags: review?(bechen) → review?(giles)
Comment on attachment 8698329 [details] [diff] [review]
Add a single CueClusterPosition to make recorded WebM video seekable.

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

Sorry I haven't been able to look at this. Matthew, do you mind taking a look while I'm away?
Attachment #8698329 - Flags: review?(giles) → review?(kinetik)
Comment on attachment 8698329 [details] [diff] [review]
Add a single CueClusterPosition to make recorded WebM video seekable.

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

Sorry, but I don't think this is the right approach.  The reason it works is that Gecko defaults to frame accurate seeking, which means that it seeks to the nearest preceeding Cluster and then decodes forward until it reaches the desired frame.  This works when writing a single Cue simply because it forces Gecko to decode *every frame* from the beginning of the media until it reaches the desired position.  That's very inefficient and it's also not behaviour that is guaranteed to be present in other WebM media players.

The preferred and (I think) right thing to do here is to write a complete set of Cues at the end of the file once recording completes, and then update the existing/pre-allocated SeekHead with the offset of the Cues.
Attachment #8698329 - Flags: review?(kinetik) → review-
(In reply to Matthew Gregan [:kinetik] from comment #25)
> The preferred and (I think) right thing to do here is to write a complete
> set of Cues at the end of the file once recording completes, and then update
> the existing/pre-allocated SeekHead with the offset of the Cues.

This was also suggested in bug 1231842 comment 3.
See Also: → 1231842
(In reply to Matthew Gregan [:kinetik] from comment #25)
> The preferred and (I think) right thing to do here is to write a complete
> set of Cues at the end of the file once recording completes, and then update
> the existing/pre-allocated SeekHead with the offset of the Cues.

By current MediaRecorder structure, ContainerWriter::GetContainerData is frequently called.
Other level 1 elements (Info, Tracks) will be extracted right after the recording starts
with the following Clusters.

If we put SeekHead before Clusters, we have no the exact position of the Cues;
if we put SeekHead after Clusters, it would be ignored by Firefox IIRC.

Within current recording mechanics
I have no idea how to update the "SeekHead before Clusters" with the offset of the "Cues after Clusters".
Maybe we need some treatment to do this right before the blob being pushed to client.
(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #26)
> (In reply to Matthew Gregan [:kinetik] from comment #25)
> > The preferred and (I think) right thing to do here is to write a complete
> > set of Cues at the end of the file once recording completes, and then update
> > the existing/pre-allocated SeekHead with the offset of the Cues.
> 
> This was also suggested in bug 1231842 comment 3.

If it's able to modify the blob data right before being pushed to client,
We could also fix some issues noted in bug 1231842 comment 3,
like duration, Segment element length, etc.
(In reply to Guang-De Lin from comment #28)
> (In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #26)
> > (In reply to Matthew Gregan [:kinetik] from comment #25)
> > > The preferred and (I think) right thing to do here is to write a complete
> > > set of Cues at the end of the file once recording completes, and then update
> > > the existing/pre-allocated SeekHead with the offset of the Cues.
> > 
> > This was also suggested in bug 1231842 comment 3.
> 
> If it's able to modify the blob data right before being pushed to client,
> We could also fix some issues noted in bug 1231842 comment 3,
> like duration, Segment element length, etc.

But is this possible when the recorded file is sliced into multiple blobs? My knowledge in mkv/webm is a bit shallow here, but it seems to me that while this might be doable with mkv segments, webm really only supports having one segment.

Perhaps it'd make more sense to move the MediaRecorder spec towards outputting blobs that are separate files instead of blobs of the same one file, and supporting playing them with the MediaSource api. Maire, has this been up for discussion before? Would it be viable?

Writing the cues in the last blob before stop sounds like a simpler short term fix though.
Flags: needinfo?(mreavy)
The mkv/webm specs both provide a lot of wiggle room to allow both fast-to-decode files where all the metadata is in the frong and streaming by processing information as it arrives.


(In reply to Matthew Gregan [:kinetik] from comment #25)
> The preferred and (I think) right thing to do here is to write a complete
> set of Cues at the end of the file once recording completes, and then update
> the existing/pre-allocated SeekHead with the offset of the Cues.

You can have cues at the end without referencing it in the seekhead. It is recommended (SHOULD) to include it where possible, but not mandatory because it's not possible when streaming.

webm spec:

> WebM SHOULD contain the SeekHead element.
> WebM files SHOULD include a keyframe-only Cues element. 
> If the Cues element is not at the beginning of the file its retrieval should be deferred to allow playback to start as soon as possible. 

Which means deferred cue processing is recommended behavior.

(In reply to Andreas Pehrson [:pehrsons] (Telenor) from comment #29)
> But is this possible when the recorded file is sliced into multiple blobs?
> My knowledge in mkv/webm is a bit shallow here, but it seems to me that
> while this might be doable with mkv segments, webm really only supports
> having one segment.

webm spec:

> Typically a Matroska file is composed of 1 segment. 

typically != mandatory

----

So I see several options here

- remove duration, append cues at the end. possibly fix decoder to pick up cues without seekhead
- emit segment-per-blob with cues-per-segment. ensure decoder can handle it
- append cues, fixup headers on a best-effort basis when MediaRecorder emits a single blob
decoder-only option: synthesize seek information as the data is downloaded
Thanks for the analysis.

I don't believe this has been discussed in the WG; it should be.

Since many of the options here involve decoder changes or potential changes, we should get the playback team's opinions on the effort and risk for the options -- and also if they think some are more likely to cause problems if the saved file is fed to other non-FF browsers.

Chris, Jean-Yves - thoughts? Preferences?  See comment 30 and comment 31
Flags: needinfo?(mreavy)
Flags: needinfo?(jyavenard)
Flags: needinfo?(cpearce)
(In reply to The 8472 from comment #30)
> You can have cues at the end without referencing it in the seekhead. It is
> recommended (SHOULD) to include it where possible, but not mandatory because
> it's not possible when streaming.

It also says:

"Muxers should treat all guidelines marked SHOULD in this section as MUST. This will foster consistency across WebM files in the real world."

And there's no efficient way I'm aware of for the decoder to find the Cues if they're at the end of the file and not included in the SeekHead.  Consider any case where access to the file is high latency (e.g. over HTTP).

But yes, that would be impossible to fulfill when streaming... sadly that's not something that was well considered when WebM or Matroska were originally specified.  It may be that we need to adjust the spec and our producers and consumers to solve this correctly.

> - remove duration, append cues at the end. possibly fix decoder to pick up
> cues without seekhead

Do you have a suggestion for how the decoder can find the Cues efficiently?

> - emit segment-per-blob with cues-per-segment. ensure decoder can handle it

It'd be interesting to survey how many WebM players can handle multiple segments.  My impression is that it's not widely supported.

(In reply to The 8472 from comment #31)
> decoder-only option: synthesize seek information as the data is downloaded

We already have most of this in place, but obviously it is restricted to seeking in the buffered portion, which may be quite limited, so it doesn't seem like an ideal solution.  I suspect it's not triggering with the data produced by media recorder because we handle HTTP and local files but not blobs.  Adding blob support would be pretty simple.
(In reply to Matthew Gregan [:kinetik] from comment #33)
> (In reply to The 8472 from comment #30)
> But yes, that would be impossible to fulfill when streaming... sadly that's
> not something that was well considered when WebM or Matroska were originally
> specified.  It may be that we need to adjust the spec and our producers and
> consumers to solve this correctly.

I think the specs considered both streaming and static files. The only aspect they didn't consider is whether anything could be done once a stream has finished and been captured to a file.

> 
> > - remove duration, append cues at the end. possibly fix decoder to pick up
> > cues without seekhead
> 
> Do you have a suggestion for how the decoder can find the Cues efficiently?

Well, it only makes sense in a limited scenario

- the file was produced in a streaming manner, i.e. segment size is not known
- the file is large/http transfer is slow, i.e. synthesizing seek information would take a long time
- range requests / file seeks are possible
- cues were appended once the stream was done

then one could load the last N% of the file and search for the ebml cues ID and run the parser to verify that it's not just a random string match.

it's an optimistic approach for limited cases. I guess the other options are more useful.


> > - emit segment-per-blob with cues-per-segment. ensure decoder can handle it
> 
> It'd be interesting to survey how many WebM players can handle multiple
> segments.  My impression is that it's not widely supported.

yeah, good point, probably restricted to players with full mkv support

> (In reply to The 8472 from comment #31)
> > decoder-only option: synthesize seek information as the data is downloaded
> 
> We already have most of this in place, but obviously it is restricted to
> seeking in the buffered portion, which may be quite limited, so it doesn't
> seem like an ideal solution.  I suspect it's not triggering with the data
> produced by media recorder because we handle HTTP and local files but not
> blobs.  Adding blob support would be pretty simple.

I guess this + attempting to fix up the blob where possible are the simplest approaches then.
Someone should fix the MediaRecorder to add Cues to the WebM files it generates, so that files recorded by Gecko are seekable in as many other players as possible.

Someone should fix Gecko's playback code to seek in files without Cues.
Flags: needinfo?(cpearce)
Assignee: nobody → bvandyk
Bryce is on this. Removing needinfo.
Flags: needinfo?(ajones)
shouldn't this be made a dupe of bug 657791?
(and bug 1232045 for mid-stream resolution change)
Depends on: 1232045, 657791
Flags: needinfo?(jyavenard)
(In reply to Jean-Yves Avenard [:jya] from comment #37)
> shouldn't this be made a dupe of bug 657791?
> (and bug 1232045 for mid-stream resolution change)

Just as cpearce said in Comment 35, I think:

Someone should fix Gecko's playback code to seek in files without Cues.
  => bug 657791
Someone should fix the MediaRecorder to add Cues to the WebM files it generates, so that files recorded by Gecko are seekable in as many other players as possible.
  => this bug or another one
Hi Bryce,  Are you still planning to fix this?   If so, do you have a rough ETA?   Thanks.
Flags: needinfo?(bvandyk)
I believe https://bugzilla.mozilla.org/show_bug.cgi?id=657791 addresses the issues noted with seeking and replay. Seeking outside of buffered ranges is not implemented, so that specific case won't work. I've tested the attached file against nightly and I am able to seek and replay.

Would you be able to test this against nightly, please, Randy?
Flags: needinfo?(bvandyk) → needinfo?(globelinmoz)
Bryce, do you have any plans to add cues to the end of recordings? Per comment 35, 38, etc.

Also, Randy left Mozilla over a year ago.
Flags: needinfo?(globelinmoz) → needinfo?(bvandyk)
On the playback side of things we've made changes that should address this (Bug 657791). Adding cues to created files is on the recording side of things, which is not something I'm involved in.

Are you having issues with this? If not I think this bug should be closed and a separate issue should be raised to track adding cues to recorded streams -- I've had a look and couldn't find a bug specifically for that.
Flags: needinfo?(bvandyk)
(In reply to Bryce Van Dyk (:SingingTree) from comment #44)
> On the playback side of things we've made changes that should address this
> (Bug 657791). Adding cues to created files is on the recording side of
> things, which is not something I'm involved in.
> 
> Are you having issues with this? If not I think this bug should be closed
> and a separate issue should be raised to track adding cues to recorded
> streams -- I've had a look and couldn't find a bug specifically for that.

OK, I'm filing a new bug.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → DUPLICATE
See Also: → 1283464
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: