Bug 237275

Created attachment 453394 [details]
Patch

Comment on attachment 453394 [details]
Patch

I am not sure about changing the GStreamer version checks; the rest of the
cleanups LGTM.

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

> Source/WebCore/platform/audio/gstreamer/AudioFileReaderGStreamer.cpp:261
> +#if GST_CHECK_VERSION(1, 19, 2)

I think it would be in the interest of Fedora users that the distribution starts
shipping the stable 1.20.0 release instead of lowering the check here in WebKit
code. These were explicitly changed to check for 1.20.0 in r289154 so I expect
there is some actual reason for that — other times we have left version checks
for release candidates in the code knowingly.

(In reply to Adrian Perez from comment #2)
> I think it would be in the interest of Fedora users that the distribution
> starts
> shipping the stable 1.20.0 release instead of lowering the check here in
> WebKit
> code. 

OK, it's a Fedora screw-up for sure. Will remove those before landing, setting cq- as a reminder for myself.

> These were explicitly changed to check for 1.20.0 in r289154 so I
> expect
> there is some actual reason for that — other times we have left version
> checks
> for release candidates in the code knowingly.

I think it's just a choice of style. I'd say it looks slightly nicer to use guards for stable versions, assuming you are OK with not supporting the relevant unstable versions.

Comment on attachment 453394 [details]
Patch

r=me, with the change that will leave the GStreamer conditionals
as they are in trunk at the moment :)

Created attachment 453540 [details]
Patch for landing

Committed r290681 (247952@main): <https://commits.webkit.org/247952@main>

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

<rdar://problem/89645743>