1057203 - Implement MP4ContainerParser::ParseStartAndEndTimestamps

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Add time stamp parsing to MP4 container parser to fix bug 1044498 for MP4.

Comment 1

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Remove MoofParser dependency on stagefright

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Parse timestamps for MP4 fragments

Comment 3

[Matthew Gregan [:kinetik]]

Comment on attachment 8477197 [details] [diff] [review]
Parse timestamps for MP4 fragments

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

::: content/media/mediasource/SourceBuffer.cpp
@@ +161,5 @@
> +
> +  virtual bool ParseStartAndEndTimestamps(const uint8_t* aData, uint32_t aLength,
> +                                          double& aStart, double& aEnd)
> +  {
> +    if (!aLength) {

Is this check needed?  Is a zero length BufferStream invalid?

@@ +181,5 @@
> +
> +    if (compositionRange.IsNull()) {
> +      return false;
> +    }
> +    aStart = compositionRange.start / 1000000.0;

Use an existing (or define a new) named constant for 1000000.0.

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

Attachment: Remove MoofParser dependency on stagefright

This is the same with BufferStream.h and BufferStream.cpp included

Comment 5

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Comment on attachment 8478076 [details] [diff] [review]
Remove MoofParser dependency on stagefright

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

::: media/libstagefright/binding/include/mp4_demuxer/BufferStream.h
@@ +8,5 @@
> +#include "mp4_demuxer/mp4_demuxer.h"
> +
> +namespace mp4_demuxer {
> +
> +class BufferStream: public Stream

Style is
class BufferStream : public Stream

@@ +11,5 @@
> +
> +class BufferStream: public Stream
> +{
> +public:
> +  BufferStream(const uint8_t* aData, size_t aLength);

Leave a comment that this class doesn't take ownership of the buffer and that it shouldn't be used past the destruction of the buffer.

Comment 6

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f595c29365b8
https://hg.mozilla.org/integration/mozilla-inbound/rev/23d73d6bed54

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f595c29365b8
https://hg.mozilla.org/mozilla-central/rev/23d73d6bed54