Closed Bug 1014393 Opened 10 years ago Closed 5 years ago

[MediaRecorder] Handle non monotonically increasing timestamp in WebM muxer.

Categories

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

Product:
Core
Component:
Audio/Video: Recording
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla70
Tracking Flags:
Tracking Status
firefox70 --- fixed
Blocking Flags:
backlog webrtc/webaudio+

People

(Reporter: bechen, Assigned: pehrsons)

References

(Depends on 1 open bug)

Details

Attachments

(15 files, 11 obsolete files)

Bug 1014393 - Break out muxing and queueing of track data from MediaEncoder to new Muxer class. r?bryce
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Add gtests for the standalone muxer. r?bryce
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Remove MediaRecorder::Session::PushBlobRunnable. r?bryce
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Remove MediaRecorder::GetParentObject. r?bryce
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Unify MediaRecorder session shutdown paths and fix event timing when stopping per spec. r?bryce
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Add Blob::CreateEmptyBlob. r?baku
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Minor mochitest fixes. r?jib
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Separate MediaEncoders encode and mux steps. r?pehrsons
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Remove EncodedFrameContainer. r?pehrsons
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Shift responsibility of adjusting packets with opus codec delay to MediaEncoder. r?pehrsons
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Update EncodedFrame class to more closely resemble MediaData class. r?pehrsons
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Use MediaQueue to store frames pending write to muxer in MediaEncoder. r?pehrsons
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - MediaEncoder better orders frames passed to the muxer. r?pehrsons
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Move AudioGenerator to its own files so it can be used in more media gtests. r?pehrsons
47 bytes, text/x-phabricator-request
Details | Review
Bug 1014393 - Use undef-after-def pattern in MediaRecorder and related files. r?bryce
47 bytes, text/x-phabricator-request
Details | Review 
Fork from bug 951040 comment 4.
http://www.webmproject.org/docs/container/#muxer-guidelines

WebM muxer should handle the timestamp which is not monotonically increasing.
Assignee: nobody → bechen
Status: NEW → ASSIGNED
Attached patch bug-1014393.patch (obsolete) — — Splinter Review 
Ensure the output cluster's timestamp is monotonically increasing.
Attachment #8430660 - Flags: feedback?(rlin)
Attachment #8430660 - Flags: feedback?(ayang)
Attached patch bug-1014393-gtest.patch (obsolete) — — Splinter Review 

        Attachment #8430662 -
        Flags: feedback?(rlin)

        Attachment #8430662 -
        Flags: feedback?(ayang)

    
    

    
  
Comment on attachment 8430660 [details] [diff] [review]
bug-1014393.patch

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

::: content/media/webm/EbmlComposer.cpp
@@ +116,5 @@
>  
> +  // TODO: Now the timestamp only occurs in video frame, maybe someday we will
> +  // need to check the timestamp for audio frame either.
> +  if ((aFrame->GetFrameType() != EncodedFrame::FrameType::VORBIS_AUDIO_FRAME) &&
> +      (aFrame->GetTimeStamp() < mClusterTimecode * PR_USEC_PER_MSEC)) {

I prefer to set a warning/assert message at this point.
That would help us find out if the encoder send wrong timestamp to muxer.

    
    

    
  
Comment on attachment 8430660 [details] [diff] [review]
bug-1014393.patch

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

::: content/media/webm/EbmlComposer.cpp
@@ +115,5 @@
>    }
>  
> +  // TODO: Now the timestamp only occurs in video frame, maybe someday we will
> +  // need to check the timestamp for audio frame either.
> +  if ((aFrame->GetFrameType() != EncodedFrame::FrameType::VORBIS_AUDIO_FRAME) &&

There are a lot of medie type in EncodedFrame::FrameType. You should specify the VP8_I_FRAME and VP8_P_FRAME here. Not assume anything not vorbis will be VP8 video.

        Attachment #8430660 -
        Flags: feedback?(ayang)

    
    

    
  
Comment on attachment 8430662 [details] [diff] [review]
bug-1014393-gtest.patch

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

::: content/media/gtest/TestWebMWriter.cpp
@@ +229,5 @@
> +
> +TEST(WebMWriter, bug1014393_monotonically_timestamp)
> +{
> +  TestWebMWriter writer(ContainerWriter::CREATE_AUDIO_TRACK |
> +                          ContainerWriter::CREATE_VIDEO_TRACK);

nit

        Attachment #8430662 -
        Flags: feedback?(ayang)

    
    

    
  
Comment on attachment 8430662 [details] [diff] [review]
bug-1014393-gtest.patch

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

LGTM, thanks for this test. :)

        Attachment #8430662 -
        Flags: feedback?(rlin) → feedback+
backlog: --- → webrtc/webaudio+
Rank: 25
Priority: -- → P2

    
    

    
  
Hey, I'm currently looking into the Media recorder and am running into this bug during my testing. Normal use seems fine, but when debugging and introducing waits (via breakpoints or slow code) this becomes quit prominent. It looks like the patches above were not merged, is that right? Would anyone (Benjamin) be opposed to me taking ownership of this?

In the immediate term I'd like to do something like in the patches above. However, given other reworks that I have in the pipeline (specifically replacing libmkv with libwebm), I would prefer to lift the timestamp sanity check into the WebMWriter.
Flags: needinfo?(bechen)

    
    

    
  
Sorry for that pending this bug for a long time. Please feel free to take this.
Flags: needinfo?(bechen)

    
    

    
  
No problem. I've been looking at this and have some WIP, so I'm going to pick this up.
Assignee: bechen → bvandyk

    
    

    
  
Comment on attachment 8430660 [details] [diff] [review]
bug-1014393.patch

Obsoleting old patches in prep for submission of new work.

        Attachment #8430660 -
        Attachment is obsolete: true

    
    

    
      
  


  

    
    

    
  
This is my WIP to address non increasing timestamps. High level explanation: this makes the MediaEncoder ensure it only gives the muxer frames that it can ensure are in order.

I've made some changes to datastructures and the code in MediaEncoder to better enable this which make up the first 2 changesets. The 3rd changeset shifts the responsibility of applying the timestamp offset from the EBML writer to the encoder. This is because the encoder needs to be aware of this offset to sync timestamps correct. The final changeset ensures sync.

I'm looking for feedback regarding these changes, particularly the high level changes, though any finer grained feedback is also appreciated.
Depends on: 1356054
Flags: needinfo?(rjesup)
Flags: needinfo?(pehrson)

    
    

    
  
The high-level changes sound good to me. Let me know if you want detailed reviews.
Flags: needinfo?(pehrson)

    
    

    
  
Mass change P2->P3 to align with new Mozilla triage process.
Priority: P2 → P3

    
    

    
      
  


  

    
    

    
      
  


  

    
    

    
      
  


  

    
    

    
      
  


  

    
    

    
      
  


  

    
    

    
  
Comment on attachment 8928570 [details]
Bug 1014393 - Separate MediaEncoders encode and mux steps.

https://reviewboard.mozilla.org/r/199798/#review204904


C/C++ static analysis found 2 defects in this patch.

You can run this analysis locally with: `./mach static-analysis check path/to/file.cpp`


::: dom/media/encoder/MediaEncoder.cpp:888
(Diff revision 1)
> +
> +  nsresult rv;
> +
> +  if (mVideoEncoder) {
> +    EncodedFrameContainer encodedVideoData;
> +    for (RefPtr<EncodedFrame> frame : mEncodedVideoFrames) {

Warning: Loop variable is copied but only used as const reference; consider making it a const reference [clang-tidy: performance-for-range-copy]

    for (RefPtr<EncodedFrame> frame : mEncodedVideoFrames) {
                              ^
         const               &

::: dom/media/encoder/MediaEncoder.cpp:905
(Diff revision 1)
> +    }
>    }
> +
> +  if (mAudioEncoder) {
> +    EncodedFrameContainer encodedAudioData;
> +    for (RefPtr<EncodedFrame> frame : mEncodedAudioFrames) {

Warning: Loop variable is copied but only used as const reference; consider making it a const reference [clang-tidy: performance-for-range-copy]

    for (RefPtr<EncodedFrame> frame : mEncodedAudioFrames) {
                              ^
         const               &

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Comment on attachment 8928570 [details]
Bug 1014393 - Separate MediaEncoders encode and mux steps.

https://reviewboard.mozilla.org/r/199798/#review205352

::: dom/media/encoder/MediaEncoder.cpp:842
(Diff revision 2)
>  {
> -  AUTO_PROFILER_LABEL("MediaEncoder::WriteEncodedDataToMuxer", OTHER);
> +  AUTO_PROFILER_LABEL("MediaEncoder::EncodeData", OTHER);
>  
>    MOZ_ASSERT(mEncoderThread->IsCurrentThreadIn());
>  
> -  if (!aTrackEncoder) {
> +  MOZ_ASSERT(mVideoEncoder || mAudioEncoder, "Must have atleast one encoder");

Make this a guard like
```
if (!mVideoEncoder && !mAudioEncoder) {
  return NS_ERROR_UNEXPECTED;
}
```

Right now in a release build when there's no encoder this would return the uninitialized `rv`.

::: dom/media/encoder/MediaEncoder.cpp:848
(Diff revision 2)
>  
> -  if (aTrackEncoder->IsEncodingComplete()) {
> -    return NS_OK;
> +  nsresult rv;
> +
> +  if (mVideoEncoder && !mVideoEncoder->IsEncodingComplete()) {
> +    EncodedFrameContainer encodedVideoData;
> +    rv = mVideoEncoder->GetEncodedTrack(encodedVideoData);

I think you should inline the `rv` decl here.

::: dom/media/encoder/MediaEncoder.cpp:862
(Diff revision 2)
> +    }
>    }
>  
> -  EncodedFrameContainer encodedData;
> -  nsresult rv = aTrackEncoder->GetEncodedTrack(encodedData);
> +  if (mAudioEncoder && !mAudioEncoder->IsEncodingComplete()) {
> +    EncodedFrameContainer encodedAudioData;
> +    rv = mAudioEncoder->GetEncodedTrack(encodedAudioData);

And here.

::: dom/media/encoder/MediaEncoder.cpp:874
(Diff revision 2)
> +         encodedAudioData.GetEncodedFrames()) {
> +      mEncodedAudioFrames.AppendElement(frame);
> +    }
> +  }
> +
> -    return rv;
> +  return rv;

This can still be uninitialized if encoders have completed. Make it return a fixed error?

::: dom/media/encoder/MediaEncoder.cpp:884
(Diff revision 2)
> +{
> +  AUTO_PROFILER_LABEL("MediaEncoder::WriteEncodedDataToMuxer", OTHER);
> +
> +  MOZ_ASSERT(mEncoderThread->IsCurrentThreadIn());
> +
> +  MOZ_ASSERT(mVideoEncoder || mAudioEncoder, "Must have atleast one encoder");

Like `EncodeData()` this doesn't cater for non-debug builds. Right now with no encoders we return NS_OK. IMHO this should be an error.

::: dom/media/encoder/MediaEncoder.cpp:889
(Diff revision 2)
> +    EncodedFrameContainer encodedVideoData;
> +    for (const RefPtr<EncodedFrame>& frame : mEncodedVideoFrames) {
> +      encodedVideoData.AppendEncodedFrame(frame);
> +    }
> +    mEncodedVideoFrames.Clear();

Give `EncodedFrameContainer` a move constructor `explicit EncodedFrameContainer(nsTArray<RefPtr<EncodedFrame>>&& aFrames);` so you don't have to do this loop-and-clear.

Well, after looking a bit more I'd rather see EncodedFrameContainer go away completely. Just replace its usage by `nsTArray<RefPtr<EncodedFrame>>`.

::: dom/media/encoder/MediaEncoder.cpp:895
(Diff revision 2)
> +    for (const RefPtr<EncodedFrame>& frame : mEncodedVideoFrames) {
> +      encodedVideoData.AppendEncodedFrame(frame);
> +    }
> +    mEncodedVideoFrames.Clear();
> +
> +    rv = mWriter->WriteEncodedTrack(

`nsresult rv = ...`

::: dom/media/encoder/MediaEncoder.cpp:899
(Diff revision 2)
> -    LOG(LogLevel::Error, ("Failed to write encoded track to the media container."));
> -    SetError();
> +      LOG(LogLevel::Error,
> +          ("Failed to write encoded video track to the muxer."));

This doesn't fit on one line?

        Attachment #8928570 -
        Flags: review?(apehrson) → review+

    
    

    
  
Comment on attachment 8928571 [details]
Bug 1014393 - Shift responsibility of adjusting packets with opus codec delay to MediaEncoder.

https://reviewboard.mozilla.org/r/199800/#review205360

Take the uint64_t -> int64_t comment as a suggestion. But if you decide to defer it, please file a bug.

::: dom/media/encoder/MediaEncoder.cpp:862
(Diff revision 2)
>      }
>    }
>  
>    if (mAudioEncoder && !mAudioEncoder->IsEncodingComplete()) {
>      EncodedFrameContainer encodedAudioData;
>      rv = mAudioEncoder->GetEncodedTrack(encodedAudioData);

(ref previous patch)
If `GetEncodedTrack()` took a `nsTArray<RefPtr<EncodedFrame>>&` we could pass it `mEncodedAudioFrames` directly :-)

::: dom/media/webm/EbmlComposer.cpp:55
(Diff revision 2)
>            }
>            // Audio
>            if (mCodecPrivateData.Length() > 0) {
>              // Extract the pre-skip from mCodecPrivateData
>              // then convert it to nanoseconds.
>              // Details in OpusTrackEncoder.cpp.

Was looking for these details but couldn't find them. Where did they go?

::: dom/media/webm/EbmlComposer.cpp:142
(Diff revision 2)
>      FinishCluster();
>      flush = true;
>    } else {
>      // Force it to calculate timecode using signed math via cast
> -    int64_t timeCode = (aFrame->GetTimeStamp() / ((int) PR_USEC_PER_MSEC) - mClusterTimecode) +
> -                       (mCodecDelay / PR_NSEC_PER_MSEC);
> +    int64_t timeCode =
> +      (aFrame->GetTimeStamp() / ((int)PR_USEC_PER_MSEC) - mClusterTimecode);

The `(int)` cast shouldn't be necessary. PR_USEC_PER_MSEG should be an `int64_t` already.

Interestingly `mClusterTimecode` and `aFrame->GetTimeStamp()` are both `uint64_t` but the  timestamps written to and read from them are always `int64_t`. Do you mind making them signed while you're here touching this?

        Attachment #8928571 -
        Flags: review?(apehrson) → review+

    
    

    
  
Comment on attachment 8928572 [details]
Bug 1014393 - Update EncodedFrame class to more closely resemble MediaData class.

https://reviewboard.mozilla.org/r/199802/#review205364

::: dom/media/encoder/EncodedFrameContainer.h:87
(Diff revision 2)
>    {
>      return mFrameData;
>    }
> -  uint64_t GetTimeStamp() const { return mTimeStamp; }
> -  void SetTimeStamp(uint64_t aTimeStamp) { mTimeStamp = aTimeStamp; }
> +  // Timestamp in microseconds
> +  uint64_t mTime;
> +  // The playback duration of this packet in number of samples or microseconds

The unit is up to user I guess. Reflect this in the comment?

::: dom/media/encoder/VP8TrackEncoder.cpp:683
(Diff revision 2)
>          mExtractedDurationUs = totalDuration;
>          if (!skippedDuration.isValid()) {
>            NS_ERROR("skipped duration overflow");
>            return NS_ERROR_DOM_MEDIA_OVERFLOW_ERR;
>          }
> -        last->SetDuration(last->GetDuration() +
> +        last->mDuration =

+=

        Attachment #8928572 -
        Flags: review?(apehrson) → review+

    
    

    
  
Comment on attachment 8928573 [details]
Bug 1014393 - MediaEncoder now uses MediaQueue to store frames pending write to muxer.

https://reviewboard.mozilla.org/r/199804/#review205366

::: commit-message-c6457:1
(Diff revision 2)
> +Bug 1014393 - MediaEncoder now uses MediaQueue to store frames pending write to muxer. r?pehrsons

This should reflect what the patch does, not what the state is before or after. I'd say "Use MediaQueue to store frames pending write to muxer in MediaEncoder"

        Attachment #8928573 -
        Flags: review?(apehrson) → review+

    
    

    
  
Comment on attachment 8928574 [details]
Bug 1014393 - MediaEncoder better orders frames passed to the muxer.

https://reviewboard.mozilla.org/r/199806/#review205370

Code looks good IMO, but this needs a unit test. Please see if you can add one.
I doubt anyone has done this with MSG before so your mileage may vary, but please give it a shot.

Getting it to work would set great precedence in unit testing the MSG so it wouldn't just be time wasted on the narrow field of MediaRecorder.

Clearing r? until we have a grip on the testing situation.

::: dom/media/encoder/MediaEncoder.cpp:915
(Diff revision 2)
> +      MOZ_ASSERT(videoFrame->mTime >= expectedNextVideoTime);
> +      MOZ_ASSERT(audioFrame->mTime >= expectedNextAudioTime);
> +      if (videoFrame->mTime <= audioFrame->mTime) {
> +        expectedNextVideoTime = videoFrame->GetEndTime();
> +        RefPtr<EncodedFrame> frame = mEncodedVideoFrames.PopFront();
> +        encodedData.AppendEncodedFrame(frame);

if `encodedData` was an nsTArray directly, you could forward the `already_AddRefed` to `AppendElement()` :-)

::: dom/media/encoder/MediaEncoder.cpp:929
(Diff revision 2)
> +        if (mAudioEncoder->IsEncodingComplete() ||
> +            videoFrame->mTime < expectedNextAudioTime) {

I like to turn these around and make them into guards. Reduces the indentation depth which is nice.

::: dom/media/encoder/MediaEncoder.cpp:968
(Diff revision 2)
>    if (mVideoEncoder) {
> +    MOZ_ASSERT(!mAudioEncoder);
>      EncodedFrameContainer encodedVideoData;
>      while (mEncodedVideoFrames.GetSize() > 0) {
>        RefPtr<EncodedFrame> frame = mEncodedVideoFrames.PopFront();
>        encodedVideoData.AppendEncodedFrame(frame);
>      }
>  
>      rv = mWriter->WriteEncodedTrack(
>        encodedVideoData,
>        mVideoEncoder->IsEncodingComplete() ? ContainerWriter::END_OF_STREAM : 0);
>      if (NS_FAILED(rv)) {
>        LOG(LogLevel::Error,
>            ("Failed to write encoded video track to the muxer."));
>        return rv;
>      }
>    }
>  
> +  // Write out our audio frames if we have an audio encoder
>    if (mAudioEncoder) {
> +    MOZ_ASSERT(!mVideoEncoder);
>      EncodedFrameContainer encodedAudioData;
>      while (mEncodedAudioFrames.GetSize() > 0) {
>        RefPtr<EncodedFrame> frame = mEncodedAudioFrames.PopFront();
>        encodedAudioData.AppendEncodedFrame(frame);
>      }
>  
>      rv = mWriter->WriteEncodedTrack(
>        encodedAudioData,
>        mAudioEncoder->IsEncodingComplete() ? ContainerWriter::END_OF_STREAM : 0);
>      if (NS_FAILED(rv)) {
>        LOG(LogLevel::Error,
>            ("Failed to write encoded audio track to the muxer."));
>        return rv;
>      }
>    }

I think you get a bit nicer code if you merged these two into
```
if ((mVideoEncoder && !mAudioEncoder) ||
    (mAudioEncoder && !mVideoEncoder)) {
  MediaEncoder* encoder = mAudioEncoder ? mAudioEncoder : mVideoEncoder;
  ...
}
```
and put them first.

Protect yourself with a `if (!mVideoEncoder && !mAudioEncoder)` too.

        Attachment #8928574 -
        Flags: review?(apehrson)

    
    

    
  
Comment on attachment 8928570 [details]
Bug 1014393 - Separate MediaEncoders encode and mux steps.

https://reviewboard.mozilla.org/r/199798/#review205352

> Give `EncodedFrameContainer` a move constructor `explicit EncodedFrameContainer(nsTArray<RefPtr<EncodedFrame>>&& aFrames);` so you don't have to do this loop-and-clear.
> 
> Well, after looking a bit more I'd rather see EncodedFrameContainer go away completely. Just replace its usage by `nsTArray<RefPtr<EncodedFrame>>`.

It shall be so, will create another changeset for the removal.

> This doesn't fit on one line?

Just a few characters too long to appease clang-format.

    
    

    
  
Comment on attachment 8928571 [details]
Bug 1014393 - Shift responsibility of adjusting packets with opus codec delay to MediaEncoder.

https://reviewboard.mozilla.org/r/199800/#review205360

> Was looking for these details but couldn't find them. Where did they go?

Good question. I had a look at the revision where this comment was landed and am not sure what it's referencing. I've updated this to link to the RFC which I think is a clearer (if not verbose) explanation of the pre-skip.

I've also added a link to this in the MediaEncoder, as I've realised the code that's moved there could benefit from it also.

> The `(int)` cast shouldn't be necessary. PR_USEC_PER_MSEG should be an `int64_t` already.
> 
> Interestingly `mClusterTimecode` and `aFrame->GetTimeStamp()` are both `uint64_t` but the  timestamps written to and read from them are always `int64_t`. Do you mind making them signed while you're here touching this?

I've removed the unneeded casts. I see that for the VP8 case we're writing an int, but for opus our frames look like they also use a uint time stamp. I would have thought it was always sane to store a uint, but that some derived times may need to be signed (block timecodes for mkv/webm). I've raised a follow up bug to investigate more in the interest of not getting side tracked at the moment: https://bugzilla.mozilla.org/show_bug.cgi?id=1417996

    
    

    
  
Comment on attachment 8928572 [details]
Bug 1014393 - Update EncodedFrame class to more closely resemble MediaData class.

https://reviewboard.mozilla.org/r/199802/#review205364

> The unit is up to user I guess. Reflect this in the comment?

Clarified the comment. The removal of the other possible types should hopefully make this clearer

    
    

    
  
Comment on attachment 8928574 [details]
Bug 1014393 - MediaEncoder better orders frames passed to the muxer.

https://reviewboard.mozilla.org/r/199806/#review205520

::: dom/media/encoder/MediaEncoder.cpp:955
(Diff revision 2)
> +          break;
> +        }
> +      }
> +    }
> +
> +    rv = mWriter->WriteEncodedTrack(encodedData, 0);

I think I've missed the case where we have two encoders, and need to handle END_OF_STREAM. Need to fix this up too.

    
    

    
  
This seems to have fallen through the cracks, is this still something we want to fix?


Flags: needinfo?(rjesup) → needinfo?(bvandyk)

    
    

    
  
Funny you should say. I'm in the progress of performing necromancy on the patches at this very moment.


Flags: needinfo?(bvandyk)

    
  

        Attachment #8928570 -
        Attachment is obsolete: true

    
  

        Attachment #8928571 -
        Attachment is obsolete: true

    
  

        Attachment #8928572 -
        Attachment is obsolete: true

    
  

        Attachment #8928573 -
        Attachment is obsolete: true

    
  

        Attachment #8928574 -
        Attachment is obsolete: true

    
    

    
  
I assume you don't mind me stealing this one after all this time.


I have performed the necessary necromancy on your old patches and added some of my own. Trying to green them up now.


Assignee: bvandyk → apehrson

    
    

    
  
Go ahead. It would be nice to finally get this one across the line!



    
    

    
  


  
This first of all does some refactoring of how metadata is encoded in

MediaEncoder. This is now guided by the new Muxer class. If we're ready to pass

data to the muxer and it does not have metadata yet, we provide metadata before

giving it any media data. This metadata is passed to the muxer in a single call.

The metadata provided in this call must stay valid for the entire recording.

This removes MediaEncoder::GetEncodedMetadata().


This also removes the ctor argument from the WebMWriter since it can now rely on

the single SetMetadata() instead.

To comply with the ContainerWriter::SetMetadata() docs,

WebMWriter::SetMetadata() will now also sanity check metadata.


ContainerWriter instances are updated somewhat, to accommodate these changes.


Lastly, and most important, the new Muxer class manages muxing of the (up to)

two tracks into a single container, ensuring that timestamps increase

monotonically throughout a recording.



    
    

    
  


  
This moves the impl of PushBlobRunnable from a runnable to MozPromise, which

let's us more easily modularize it's parts (gather the blob, fire dataavailable)

to make individual code paths more explicit.



    
    

    
  


  
I needed empty blobs in MediaRecorder and seeing that there was no

Blob::CreateEmptyBlob, nor an export of EmptyBlobImpl, I thought

Blob::CreateEmptyBlob would be the cleaner solution, so here it is.


Depends on D17814



    
    

    
  


  
Depends on D35310



    
    

    
  
This is still lacking some of Bryce's patches (that I wasn't able to upload to phabricator, since I'm also the reviewer). While waiting for him to push those, let's marvel at a green try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f98d99b43e9c8598fb5bf9594b3ef869b19c5d70



    
    

    
  


  
Separating the encode and mux steps allows for better control over interleaving

audio and video data. If encode and mux are done in a single step it's possible

to mux large amounts of audio or video data which should have been interleaved

with the other data type to give correctly ordered time stamps in the target

container.


MozReview-Commit-ID: CBYemrkpyF5



    
    

    
  


  
Remove EncodedFrameContainer and clean up areas where it was used.

EncodedFrameContainer provided a wrapper around an

nsTArray<RefPtr<EncodedFrame>>, but it simplifies the code to simply expose

this array. Also clean up unused enums in EncodedFrame, and clean up some of

the outdated comments for our encoded frame handling.


MozReview-Commit-ID: Bh3VKesVoJE


Depends on D35383



    
    

    
  


  
Move the responsibility of adjusting opus frame timestamps to the MediaEncoder.

This was previously done by the EbmlComposer, but doing so in the MediaEncoder

means we can have greater control over handling of time codes and interleaving

of frames.


MozReview-Commit-ID: 2g9cy1IqOph


Depends on D35384



    
    

    
  


  
This changes EncodedFrame to behave more like MediaData, so that EncodedFrame

can be used with the MediaQueue data structure. It also provides a somewhat

more consistent interface across media data types.


MozReview-Commit-ID: I2o6n30ErxB


Depends on D35385



    
    

    
  


  
MediaQueue provides a better interface for interleaving frames when writing to

the muxer (this change will follow in another changeset). The queue interface

provides a nicer abstraction than manually managing a nsTArray.


MozReview-Commit-ID: 5V5XmYODFdA


Depends on D35386



    
    

    
  


  
Update MediaEncoder to pass frames to the muxer in order of their time stamps.

This should prevent the currently possible scenario where audio and video

frames are written with non-monotonically increasing timestamps (in violation

of the webm spec).


Depends on D35387



    
  
Depends on: 1523563

        Attachment #9077350 -
        Attachment description: Bug 1014393 Use undef-after-def pattern in MediaRecorder and related files. r?bryce → Bug 1014393 - Use undef-after-def pattern in MediaRecorder and related files. r?bryce

    
    

    
  
This couldn't be landed cleanly. I'll see if I'm able to push rebased versions of bryce's patches.



    
    

    
  
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/25f58baa8159
Separate MediaEncoders encode and mux steps. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/d17f91ff2014
Remove EncodedFrameContainer. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/234994d8f136
Shift responsibility of adjusting packets with opus codec delay to MediaEncoder. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/0be3804dc04e
Update EncodedFrame class to more closely resemble MediaData class. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/2a4999e0475b
Use MediaQueue to store frames pending write to muxer in MediaEncoder. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/3457a5065dc4
MediaEncoder better orders frames passed to the muxer. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/5a01d6d44634
Move AudioGenerator to its own files so it can be used in more media gtests. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/e9416b502170
Break out muxing and queueing of track data from MediaEncoder to new Muxer class. r=bryce
https://hg.mozilla.org/integration/autoland/rev/2f030ee55722
Add gtests for the standalone muxer. r=bryce
https://hg.mozilla.org/integration/autoland/rev/72e545bdcce3
Remove MediaRecorder::Session::PushBlobRunnable. r=bryce
https://hg.mozilla.org/integration/autoland/rev/6755ef7b361c
Remove MediaRecorder::GetParentObject. r=bryce
https://hg.mozilla.org/integration/autoland/rev/2812ca77d87d
Unify MediaRecorder session shutdown paths and fix event timing when stopping per spec. r=bryce
https://hg.mozilla.org/integration/autoland/rev/fe652475a785
Add Blob::CreateEmptyBlob. r=baku
https://hg.mozilla.org/integration/autoland/rev/37399c83d3bf
Minor mochitest fixes. r=jib
https://hg.mozilla.org/integration/autoland/rev/0ddbdbfa87f6
Use undef-after-def pattern in MediaRecorder and related files. r=bryce

    
    

    
  
Backed out 15 changesets for causing bustages in TestMuxer.cpp and gmock-internal-utils.h


Backout link: https://hg.mozilla.org/integration/autoland/rev/1457e831fb67d5cfe8d9de391a075a281a1a705d


Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cusercancel%2Crunnable&fromchange=0ddbdbfa87f6bb5ca963f77e6adc86739566fc04&tochange=1457e831fb67d5cfe8d9de391a075a281a1a705d&selectedJob=256203104


Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256203104&repo=autoland&lineNumber=23895


[task 2019-07-12T14:06:24.038Z] 14:06:24     INFO -  /builds/worker/workspace/build/src/sccache/sccache /builds/worker/workspace/build/src/gcc/bin/g++ -o Unified_cpp_dom_media_gtest1.o -c  -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DHAVE_UINT64_T -DWEBRTC_POSIX -DWEBRTC_BUILD_LIBEVENT -DWEBRTC_LINUX -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/dom/media/gtest -I/builds/worker/workspace/build/src/obj-firefox/dom/media/gtest -I/builds/worker/workspace/build/src/media/webrtc/signaling/src/common -I/builds/worker/workspace/build/src/media/webrtc/trunk -I/builds/worker/workspace/build/src/media/webrtc/trunk/webrtc -I/builds/worker/workspace/build/src/gfx/2d -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/dom/media -I/builds/worker/workspace/build/src/dom/media/encoder -I/builds/worker/workspace/build/src/dom/media/gmp -I/builds/worker/workspace/build/src/dom/media/mp4 -I/builds/worker/workspace/build/src/dom/media/platforms -I/builds/worker/workspace/build/src/dom/media/platforms/agnostic -I/builds/worker/workspace/build/src/security/certverifier -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O2 -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -Wno-error=shadow  -MD -MP -MF .deps/Unified_cpp_dom_media_gtest1.o.pp   /builds/worker/workspace/build/src/obj-firefox/dom/media/gtest/Unified_cpp_dom_media_gtest1.cpp

[task 2019-07-12T14:06:24.038Z] 14:06:24     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/dom/media/gtest/Unified_cpp_dom_media_gtest1.cpp:110:0:

[task 2019-07-12T14:06:24.038Z] 14:06:24    ERROR -  /builds/worker/workspace/build/src/dom/media/gtest/TestMuxer.cpp:74:7: error: specialization of 'template<class RawContainer> class testing::internal::StlContainerView' in different namespace [-fpermissive]

[task 2019-07-12T14:06:24.038Z] 14:06:24     INFO -   class StlContainerView<nsTArray<Element>> {

[task 2019-07-12T14:06:24.038Z] 14:06:24     INFO -         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[task 2019-07-12T14:06:24.038Z] 14:06:24     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/dist/include/gmock/gmock-actions.h:47:0,

[task 2019-07-12T14:06:24.039Z] 14:06:24     INFO -                   from /builds/worker/workspace/build/src/obj-firefox/dist/include/gmock/gmock.h:59,

[task 2019-07-12T14:06:24.039Z] 14:06:24     INFO -                   from /builds/worker/workspace/build/src/dom/media/gtest/TestGroupId.cpp:8,

[task 2019-07-12T14:06:24.039Z] 14:06:24     INFO -                   from /builds/worker/workspace/build/src/obj-firefox/dom/media/gtest/Unified_cpp_dom_media_gtest1.cpp:29:

[task 2019-07-12T14:06:24.039Z] 14:06:24    ERROR -  /builds/worker/workspace/build/src/obj-firefox/dist/include/gmock/internal/gmock-internal-utils.h:434:7: error:   from definition of 'template<class RawContainer> class testing::internal::StlContainerView' [-fpermissive]

[task 2019-07-12T14:06:24.039Z] 14:06:24     INFO -   class StlContainerView {

[task 2019-07-12T14:06:24.039Z] 14:06:24     INFO -         ^~~~~~~~~~~~~~~~

[task 2019-07-12T14:06:24.039Z] 14:06:24     INFO -  /builds/worker/workspace/build/src/config/rules.mk:801: recipe for target 'Unified_cpp_dom_media_gtest1.o' failed

[task 2019-07-12T14:06:24.039Z] 14:06:24    ERROR -  make[4]: *** [Unified_cpp_dom_media_gtest1.o] Error 1

[task 2019-07-12T14:06:24.039Z] 14:06:24     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/gtest'

[task 2019-07-12T14:06:24.040Z] 14:06:24     INFO -  make[4]: *** Waiting for unfinished jobs....

[task 2019-07-12T14:06:24.040Z] 14:06:24     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/svg'

[task 2019-07-12T14:06:24.040Z] 14:06:24     INFO -  dom/svg/Unified_cpp_dom_svg2.o

[task 2019-07-12T14:06:24.040Z] 14:06:24     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/svg'


Flags: needinfo?(apehrson)

    
    

    
  
Thanks. I'll look at this after my PTO.


Flags: needinfo?(apehrson)

    
    

    
  
Added log for the MDA failures that were happening because of this.


https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256220234&repo=autoland&lineNumber=3325



    
    

    
  
Because of a typo I missed running mochitest-media tests in the final try run before the push, and that's where the issues were found. It is good now. I'm just giving bug 1493613 some time to settle before pushing this later.



    
    

    
  
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b4fb228c8624
Separate MediaEncoders encode and mux steps. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/e872361b9f3b
Remove EncodedFrameContainer. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/085fb3e97aaa
Shift responsibility of adjusting packets with opus codec delay to MediaEncoder. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/aa508d83a2c3
Update EncodedFrame class to more closely resemble MediaData class. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/bf46f2a2e519
Use MediaQueue to store frames pending write to muxer in MediaEncoder. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/a16bcc633362
MediaEncoder better orders frames passed to the muxer. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/8bcbc931942b
Move AudioGenerator to its own files so it can be used in more media gtests. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/12f932aeb3e9
Break out muxing and queueing of track data from MediaEncoder to new Muxer class. r=bryce
https://hg.mozilla.org/integration/autoland/rev/0e85d193f8ff
Add gtests for the standalone muxer. r=bryce
https://hg.mozilla.org/integration/autoland/rev/24f244ea8fff
Remove MediaRecorder::Session::PushBlobRunnable. r=bryce
https://hg.mozilla.org/integration/autoland/rev/b88188e0d590
Remove MediaRecorder::GetParentObject. r=bryce
https://hg.mozilla.org/integration/autoland/rev/0dd76879ff03
Unify MediaRecorder session shutdown paths and fix event timing when stopping per spec. r=bryce
https://hg.mozilla.org/integration/autoland/rev/9a087b7459cc
Add Blob::CreateEmptyBlob. r=baku
https://hg.mozilla.org/integration/autoland/rev/2d880ea6e67c
Minor mochitest fixes. r=jib
https://hg.mozilla.org/integration/autoland/rev/e6d834f3786e
Use undef-after-def pattern in MediaRecorder and related files. r=bryce

    
    

    
  
Backed out 15 changesets (bug 1014393) for build bustage at TestMuxer.cpp on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/b5f976c733bc181e3519b923a1689fede65c06e2


**Push with failures:**https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=e6d834f3786e0802f2a550e4d2a64e28d8b29741&selectedJob=259250761


Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=259250761&repo=autoland&lineNumber=21118


Log snippet:

[task 2019-07-31T20:14:25.633Z] 20:14:25  WARNING -  /builds/worker/workspace/build/src/dom/media/webspeech/synth/SpeechSynthesisVoice.cpp:60:10: warning: 'isLocal' may be used uninitialized in this function [-Wmaybe-uninitialized]

[task 2019-07-31T20:14:25.633Z] 20:14:25     INFO -     return isLocal;

[task 2019-07-31T20:14:25.633Z] 20:14:25     INFO -            ^~~~~~~

[task 2019-07-31T20:14:25.633Z] 20:14:25     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/webspeech/synth'

[task 2019-07-31T20:14:25.633Z] 20:14:25     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/network'

[task 2019-07-31T20:14:25.633Z] 20:14:25     INFO -  mkdir -p '.deps/'

[task 2019-07-31T20:14:25.634Z] 20:14:25     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/network'

[task 2019-07-31T20:14:25.634Z] 20:14:25     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/network'

[task 2019-07-31T20:14:25.634Z] 20:14:25     INFO -  dom/network/Unified_cpp_dom_network0.o

[task 2019-07-31T20:14:25.634Z] 20:14:25     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/network'

[task 2019-07-31T20:14:29.807Z] 20:14:29     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/gtest'

[task 2019-07-31T20:14:29.808Z] 20:14:29     INFO -  /builds/worker/workspace/build/src/sccache/sccache /builds/worker/workspace/build/src/gcc/bin/g++ -o Unified_cpp_dom_media_gtest1.o -c  -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DHAVE_UINT64_T -DWEBRTC_POSIX -DWEBRTC_BUILD_LIBEVENT -DWEBRTC_LINUX -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/dom/media/gtest -I/builds/worker/workspace/build/src/obj-firefox/dom/media/gtest -I/builds/worker/workspace/build/src/media/webrtc/signaling/src/common -I/builds/worker/workspace/build/src/media/webrtc/trunk -I/builds/worker/workspace/build/src/media/webrtc/trunk/webrtc -I/builds/worker/workspace/build/src/gfx/2d -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/dom/media -I/builds/worker/workspace/build/src/dom/media/encoder -I/builds/worker/workspace/build/src/dom/media/gmp -I/builds/worker/workspace/build/src/dom/media/mp4 -I/builds/worker/workspace/build/src/dom/media/platforms -I/builds/worker/workspace/build/src/dom/media/platforms/agnostic -I/builds/worker/workspace/build/src/security/certverifier -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O2 -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -Wno-error=shadow  -MD -MP -MF .deps/Unified_cpp_dom_media_gtest1.o.pp   /builds/worker/workspace/build/src/obj-firefox/dom/media/gtest/Unified_cpp_dom_media_gtest1.cpp

[task 2019-07-31T20:14:29.809Z] 20:14:29     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/dom/media/gtest/Unified_cpp_dom_media_gtest1.cpp:110:0:

[task 2019-07-31T20:14:29.809Z] 20:14:29    ERROR -  /builds/worker/workspace/build/src/dom/media/gtest/TestMuxer.cpp:74:7: error: specialization of 'template<class RawContainer> class testing::internal::StlContainerView' in different namespace [-fpermissive]

[task 2019-07-31T20:14:29.809Z] 20:14:29     INFO -   class StlContainerView<nsTArray<Element>> {

[task 2019-07-31T20:14:29.810Z] 20:14:29     INFO -         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[task 2019-07-31T20:14:29.810Z] 20:14:29     INFO -  In file included from /builds/worker/workspace/build/src/obj-firefox/dist/include/gmock/gmock-actions.h:47:0,

[task 2019-07-31T20:14:29.810Z] 20:14:29     INFO -                   from /builds/worker/workspace/build/src/obj-firefox/dist/include/gmock/gmock.h:59,

[task 2019-07-31T20:14:29.810Z] 20:14:29     INFO -                   from /builds/worker/workspace/build/src/dom/media/gtest/TestGroupId.cpp:8,

[task 2019-07-31T20:14:29.810Z] 20:14:29     INFO -                   from /builds/worker/workspace/build/src/obj-firefox/dom/media/gtest/Unified_cpp_dom_media_gtest1.cpp:29:

[task 2019-07-31T20:14:29.810Z] 20:14:29    ERROR -  /builds/worker/workspace/build/src/obj-firefox/dist/include/gmock/internal/gmock-internal-utils.h:434:7: error:   from definition of 'template<class RawContainer> class testing::internal::StlContainerView' [-fpermissive]

[task 2019-07-31T20:14:29.810Z] 20:14:29     INFO -   class StlContainerView {

[task 2019-07-31T20:14:29.810Z] 20:14:29     INFO -         ^~~~~~~~~~~~~~~~

[task 2019-07-31T20:14:29.810Z] 20:14:29     INFO -  /builds/worker/workspace/build/src/config/rules.mk:786: recipe for target 'Unified_cpp_dom_media_gtest1.o' failed

[task 2019-07-31T20:14:29.811Z] 20:14:29    ERROR -  make[4]: *** [Unified_cpp_dom_media_gtest1.o] Error 1

[task 2019-07-31T20:14:29.811Z] 20:14:29     INFO -  make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/media/gtest'

[task 2019-07-31T20:14:29.811Z] 20:14:29     INFO -  make[4]: *** Waiting for unfinished jobs....

[task 2019-07-31T20:14:29.811Z] 20:14:29     INFO -  make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/svg'

[task 2019-07-31T20:14:29.811Z] 20:14:29     INFO -  dom/svg/Unified_cpp_dom_svg1.o


Flags: needinfo?(apehrson)

    
    

    
  
There's always some job you miss to run on try, apparently.


Thanks for the backout, I'll try to figure this out.


Flags: needinfo?(apehrson)

    
    

    
  
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/c58e17df9c99
Separate MediaEncoders encode and mux steps. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/147d5aeaab46
Remove EncodedFrameContainer. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/4a57b865b461
Shift responsibility of adjusting packets with opus codec delay to MediaEncoder. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/4b11f19aa613
Update EncodedFrame class to more closely resemble MediaData class. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/fd846ac16731
Use MediaQueue to store frames pending write to muxer in MediaEncoder. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/fc24872739e4
MediaEncoder better orders frames passed to the muxer. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/c49241bad727
Move AudioGenerator to its own files so it can be used in more media gtests. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/f9a9b2fc3335
Break out muxing and queueing of track data from MediaEncoder to new Muxer class. r=bryce
https://hg.mozilla.org/integration/autoland/rev/579d7f15d4f2
Add gtests for the standalone muxer. r=bryce
https://hg.mozilla.org/integration/autoland/rev/5c4b5620be2e
Remove MediaRecorder::Session::PushBlobRunnable. r=bryce
https://hg.mozilla.org/integration/autoland/rev/e35a1a354bb5
Remove MediaRecorder::GetParentObject. r=bryce
https://hg.mozilla.org/integration/autoland/rev/ea6314a61a77
Unify MediaRecorder session shutdown paths and fix event timing when stopping per spec. r=bryce
https://hg.mozilla.org/integration/autoland/rev/21ec9e104912
Add Blob::CreateEmptyBlob. r=baku
https://hg.mozilla.org/integration/autoland/rev/be1f1f82f92c
Minor mochitest fixes. r=jib
https://hg.mozilla.org/integration/autoland/rev/83a1758bc6fa
Use undef-after-def pattern in MediaRecorder and related files. r=bryce
Regressions: 1570958

    
  
No longer blocks: 1569645

    
    

    
  
Backed out 15 changesets (bug 1014393) for permafailing at test_mediarecorder_record_gum_video_timeslice_mixed.html a=backout


Backout link: https://hg.mozilla.org/mozilla-central/rev/6954782553c76545a6ac10a2f5373e17b5e577c7


Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=success%2Cpending%2Crunning%2Ctestfailed%2Cbusted%2Cexception&searchStr=android%2Cmda&tochange=470fee6b609006fbab428baff5a901bf69d20e65&fromchange=320e196f42d3839cdb91f1e3fc04ceaea1f24748


Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=259781494&repo=autoland&lineNumber=3779


Log snippet: task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  Buffered messages logged at 15:52:30

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  665 INFO dataavailable fired, size=0, memory=true, file=false

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  666 INFO dataavailable fired, size=2766, memory=true, file=false

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  667 INFO dataavailable fired, size=0, memory=true, file=false

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  668 INFO dataavailable fired, size=0, memory=true, file=false

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  669 INFO dataavailable fired, size=2697, memory=true, file=false

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  670 INFO dataavailable fired, size=0, memory=true, file=false

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  671 INFO dataavailable fired, size=0, memory=true, file=false

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  672 INFO dataavailable fired, size=2837, memory=true, file=false

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  Buffered messages finished

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  673 INFO TEST-UNEXPECTED-FAIL | dom/media/test/test_mediarecorder_record_gum_video_timeslice_mixed.html | Test timed out.

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  SimpleTest.ok@https://example.com/tests/SimpleTest/SimpleTest.js:275:18

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  reportError@https://example.com/tests/SimpleTest/TestRunner.js:121:22

[task 2019-08-03T15:52:34.998Z] 15:52:34     INFO -  TestRunner._checkForHangs@https://example.com/tests/SimpleTest/TestRunner.js:142:7


Status: RESOLVED → REOPENED

          status-firefox70:
          fixed → ---
Flags: needinfo?(apehrson)
Resolution: FIXED → ---
Target Milestone: mozilla70 → ---

    
    

    
  
Backout by apavel@mozilla.com:
https://hg.mozilla.org/mozilla-central/rev/3f71769e7099
Backed out changeset 6954782553c7 for build bustages a=backout
Regressions: 1517022

    
  
Flags: needinfo?(apehrson)

    
    

    
  
Pushed by pehrsons@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a010340cfa03
Separate MediaEncoders encode and mux steps. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/ca958984393b
Remove EncodedFrameContainer. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/1af87ed9c5fb
Shift responsibility of adjusting packets with opus codec delay to MediaEncoder. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/01204a8c60a4
Update EncodedFrame class to more closely resemble MediaData class. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/c979aee45aab
Use MediaQueue to store frames pending write to muxer in MediaEncoder. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/d099c60bfd46
MediaEncoder better orders frames passed to the muxer. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/3184a33c1daa
Move AudioGenerator to its own files so it can be used in more media gtests. r=pehrsons
https://hg.mozilla.org/integration/autoland/rev/74955151edc0
Break out muxing and queueing of track data from MediaEncoder to new Muxer class. r=bryce
https://hg.mozilla.org/integration/autoland/rev/829c780e6595
Add gtests for the standalone muxer. r=bryce
https://hg.mozilla.org/integration/autoland/rev/ba8ce9f1e1b7
Remove MediaRecorder::Session::PushBlobRunnable. r=bryce
https://hg.mozilla.org/integration/autoland/rev/9c4e9293665c
Remove MediaRecorder::GetParentObject. r=bryce
https://hg.mozilla.org/integration/autoland/rev/0674f2a4ad8e
Unify MediaRecorder session shutdown paths and fix event timing when stopping per spec. r=bryce
https://hg.mozilla.org/integration/autoland/rev/ba5ccd1f7313
Add Blob::CreateEmptyBlob. r=baku
https://hg.mozilla.org/integration/autoland/rev/791ab650c228
Minor mochitest fixes. r=jib
https://hg.mozilla.org/integration/autoland/rev/c62066a6c4b1
Use undef-after-def pattern in MediaRecorder and related files. r=bryce

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: