Bug 222462 – [REGRESSION] [MSE] WebContent/GPU process will hang when appending data under some circumstances.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jean-Yves Avenard [:jya]]

[REGRESSION] [MSE] WebContent/GPU process will hang when appending data under some circumstances.

[Jean-Yves Avenard [:jya]]

rdar://74748843

[Jean-Yves Avenard [:jya]]

Created attachment 421620 [details]
Patch

[Jer Noble]

Comment on attachment 421620 [details]
Patch

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

> Source/WebCore/platform/MediaSample.h:76
> -    virtual std::pair<RefPtr<MediaSample>, RefPtr<MediaSample>> divide(const MediaTime& presentationTime) = 0;
> +    virtual std::pair<RefPtr<MediaSample>, RefPtr<MediaSample>> divide(const MediaTime& presentationTime, bool useEndTime = false) = 0;

Here...

> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:1020
> +                    replacementSamples = replacementSamples.second->divide(m_appendWindowEnd, true /* useEndTime */);

And here:

Rather than have a comment indicate the meaning of "true" in this case, something we've done elsewhere is:

enum class UseEndTime : bool { 
  DoNotUse,
  Use,
};

and: 

virtual std::pair<RefPtr<MediaSample>, RefPtr<MediaSample>> divide(const MediaTime& presentationTime, UseEndTime = UseEndTime::DoNotUse);

and:

replacementSamples = replacementSamples.second->divide(m_appendWindowEnd, UseEndTime::Use);

[Eric Carlson]

Comment on attachment 421620 [details]
Patch

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

> Source/WebCore/platform/graphics/SourceBufferPrivate.cpp:1020
> +                    replacementSamples = replacementSamples.second->divide(m_appendWindowEnd, true /* useEndTime */);

Please define and use an enum here instead of a bool

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.h:64
> +    std::pair<RefPtr<MediaSample>, RefPtr<MediaSample>> divide(const MediaTime& presentationTime, bool useEndTime) override;

s/override/final/

> Source/WebCore/platform/graphics/gstreamer/MediaSampleGStreamer.h:53
> +    std::pair<RefPtr<MediaSample>, RefPtr<MediaSample>> divide(const MediaTime&, bool) override  { return { nullptr, nullptr }; }

Ditto.

> Source/WebCore/platform/mock/mediasource/MockSourceBufferPrivate.cpp:71
> +    std::pair<RefPtr<MediaSample>, RefPtr<MediaSample>> divide(const MediaTime&, bool) override { return {nullptr, nullptr}; }

Ditto.

[Jean-Yves Avenard [:jya]]

Created attachment 421707 [details]
Patch

[Jer Noble]

Comment on attachment 421707 [details]
Patch

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

r=me, with nits before landing:

> Source/WebCore/platform/MediaSample.h:79
> +    enum class UseEndTime : bool {
> +      DoNotUse,
> +      Use,
> +    };

StyleBot is complaining that the enum values aren't indented correctly.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaSampleAVFObjC.h:64
> -    std::pair<RefPtr<MediaSample>, RefPtr<MediaSample>> divide(const MediaTime& presentationTime) override;
> +    std::pair<RefPtr<MediaSample>, RefPtr<MediaSample>> divide(const MediaTime& presentationTime, UseEndTime useEndTime) override;

StyleBot is complaining that the parameter name isn't needed here.

[Jean-Yves Avenard [:jya]]

Created attachment 421719 [details]
Patch

[EWS]

Committed r273604: <https://commits.webkit.org/r273604>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 421719 [details].