Bug 238685

Summary: [GStreamer] gst_video_format_info_component not defined in GStreamer <1.18
Product: WebKit Reporter: Diego Pino <dpino>
Component: New BugsAssignee: Diego Pino <dpino>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, pnormand, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Diego Pino 2022-04-01 16:18:10 PDT
[GStreamer] gst_video_format_info_component not defined in GStreamer <1.18
Comment 1 Diego Pino 2022-04-01 16:19:21 PDT
Created attachment 456416 [details]
Patch
Comment 2 Philippe Normand 2022-04-02 00:35:09 PDT
Comment on attachment 456416 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:-33
> +#if USE(GSTREAMER)
> +#include "GStreamerCommon.h"
> +#endif
> +
>  #if ENABLE(VIDEO) && USE(GSTREAMER)
>  
>  #include "GraphicsContext.h"
>  #include "GStreamerAudioMixer.h"
> -#include "GStreamerCommon.h"

Why is this change needed?
Comment 3 Xabier Rodríguez Calvar 2022-04-04 01:26:43 PDT
Comment on attachment 456416 [details]
Patch

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

> Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.cpp:93
> +    /* Reverse mapping of info->plane */

. at the end as per coding style.

>> Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:-33
>> -#include "GStreamerCommon.h"
> 
> Why is this change needed?

Please, do not remove this.
Comment 4 Diego Pino 2022-04-04 08:50:16 PDT
Created attachment 456574 [details]
Patch
Comment 5 Philippe Normand 2022-04-04 09:14:37 PDT
Comment on attachment 456574 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        No new tests (OOPS!).

You can remove this and then we're good to go!
Comment 6 Diego Pino 2022-04-05 00:26:40 PDT
Created attachment 456679 [details]
Patch
Comment 7 Diego Pino 2022-04-05 00:30:59 PDT
(In reply to Philippe Normand from comment #2)
> Comment on attachment 456416 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=456416&action=review
> 
> > Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamer.cpp:-33
> > +#if USE(GSTREAMER)
> > +#include "GStreamerCommon.h"
> > +#endif
> > +
> >  #if ENABLE(VIDEO) && USE(GSTREAMER)
> >  
> >  #include "GraphicsContext.h"
> >  #include "GStreamerAudioMixer.h"
> > -#include "GStreamerCommon.h"
> 
> Why is this change needed?

The reason why I initially introduced this change was because at some point I got a link problem. The linker could not resolve the symbol `webkitGstVideoFormatInfoComponent`. I implemented this change and got the link error solved. However, after the feedback, I undid the change and build from scratch WebKitGTK/WPE and got no link error, so it seemed this change was not needed.
Comment 8 EWS 2022-04-05 02:12:50 PDT
Committed r292387 (249252@main): <https://commits.webkit.org/249252@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456679 [details].
Comment 9 Radar WebKit Bug Importer 2022-04-05 02:13:17 PDT
<rdar://problem/91283863>