[meta-xilinx] [dev-2017.1] gstreamer1.0-plugins-bad related bbapend file and patches.

[Nathan Rossi]

On 3 February 2017 at 23:10, Dhaval Rajeshbhai Shah
<dhaval.shah at xilinx.com> wrote:
> Hi Nathan,
>
> This patches are for the dev-3017.1 branch of the meta-xilinx repo.
>
> To remove the confusion, I have updated the subject as well.

In the future to avoid such confusion of this matter please avoid
sending patches for any Xilinx customized fork of meta-xilinx to this
list (meta-xilinx at lists.yoctoproject.org). Unless you intend for the
changes to be applied to meta-xilinx as well.

>
> This changes are to integrate the bad plugin related changes which are not
> part of the bad plugin bb file.
>
> Some of the patches contains the changed specific to Xilinx so that don't
> need to be upstream to where gstreamer bad plugin related changes available.
> But need to maintain in the Xilinx repository meta-xilinx as patch.

Just cause your changes are "Xilinx" specific does not make them
invalid for submission upstream.You should be upstreaming your changes
(or at least trying to) before trying to put your patches in down
stream projects like Yocto/OE/meta-xilinx. Otherwise you will be
spending more time patching gstreamer in projects like meta-xilinx,
buildroot, Ubuntu, etc. And all because you "don't need to" upstream
your changes to gstreamer.

Regards,
Nathan

>
> Thanks & Regards,
>
> Dhaval
>
> -------- Original Message --------
>
> Subject: Re: [meta-xilinx] [metx-xilinx][dev-2017.1]
> gstreamer1.0-plugins-bad related bbapend file and patches.
>
> From: Nathan Rossi <nathan at nathanrossi.com>
>
> Date: Feb 3, 2017, 6:09 PM
>
> To: Dhaval Rajeshbhai Shah <DSHAH at xilinx.com>
>
> On 3 February 2017 at 21:39, Dhaval Shah <dhaval.shah at xilinx.com> wrote:
>> All the patches from the 2016.4 maintained earlier as locally.
>> Now, ported to the 2017.1 and related bbapend file is also added.
>
> Hi Dhaval,
>
> So I am a little confused. Are these patches for meta-xilinx? or are
> these internal patches? or something else entirely? My below comments
> assume your intention was to have this in meta-xilinx master.
>
> If these are for meta-xilinx are they intended for master? because
> oe-core already has gstreamer 1.10.2, which makes a number of the
> backported patches unnecessary.
>
> Also "Never" is not a standard Upstream-Status value (I know Manju
> said it was in his other email).
> http://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines#Patch_Header_Recommendations
>
> However if it is intended to never submit the patches upstream you
> will need to provide a reason why, secondly this reason must provide a
> good explanation why meta-xilinx should maintain the patches, since by
> not upstreaming to gstreamer you are placing the maintenance burden in
> meta-xilinx.
>
> Also note, some of the comments for this patch also apply as comments
> for your other patch -omx bbappend.
>
>>
>> Signed-off-by: Dhaval Shah <dshah at xilinx.com>
>> ---
>>  ...1-gst-plugins-bad-Copy-kmssink-from-1.9.2.patch | 2550
>> ++++++++++++++++++++
>>  .../0002-Compile-kms.patch                         |   80 +
>>  ...03-gst-kmssink-Add-support-for-xilinx-drm.patch |   31 +
>>  ...sink-override-stride-if-defined-in-driver.patch |   54 +
>>  ...05-kmssink-Fix-selection-of-source-region.patch |   88 +
>>  ...-kmssink-Scale-up-to-the-screen-dimension.patch |   31 +
>>  .../0007-kmssink-experimentation.patch             |   89 +
>>  .../gstreamer/gstreamer1.0-plugins-bad_%.bbappend  |   18 +
>>  8 files changed, 2941 insertions(+)
>>  create mode 100644
>> recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0001-gst-plugins-bad-Copy-kmssink-from-1.9.2.patch
>>  create mode 100644
>> recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0002-Compile-kms.patch
>>  create mode 100644
>> recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0003-gst-kmssink-Add-support-for-xilinx-drm.patch
>>  create mode 100644
>> recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0004-kmssink-override-stride-if-defined-in-driver.patch
>>  create mode 100644
>> recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0005-kmssink-Fix-selection-of-source-region.patch
>>  create mode 100644
>> recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0006-kmssink-Scale-up-to-the-screen-dimension.patch
>>  create mode 100644
>> recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad/0007-kmssink-experimentation.patch
>>  create mode 100644
>> recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend
>>
>
> ... snip ...
>
>> +--
>> +2.7.4
>> +
>> diff --git
>> a/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend
>> b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend
>> new file mode 100644
>> index 0000000..463cebd
>> --- /dev/null
>> +++ b/recipes-multimedia/gstreamer/gstreamer1.0-plugins-bad_%.bbappend
>> @@ -0,0 +1,18 @@
>> +PACKAGECONFIG_GL = "${@bb.utils.contains('DISTRO_FEATURES', 'opengl', '
>> opengl gles2', '', d)}"
>
> Why? also why does it need to be a global change? affecting every
> machine/board/distro/etc. (when meta-xilinx is in bblayers)?
>
> Also a bit confused here since ZynqMP does not have opengl hardware
> acceleration, only opengl es...?
>
>> +PACKAGECONFIG_append = "faad"
>
> This looks like distro config not bsp config? Also this introduces a
> dependency on meta-openembedded since "faad2" is not in oe-core. Why
> is this option needed?
>
>> +
>> +FILESEXTRAPATHS_prepend := "${THISDIR}/gstreamer1.0-plugins-bad:"
>> +
>> +#
>> +# Need to make this conditional to gstreamer1
>> +#
>> +SRC_URI_append_zynqmp = " \
>> +    file://0001-gst-plugins-bad-Copy-kmssink-from-1.9.2.patch \
>> +    file://0002-Compile-kms.patch \
>> +    file://0003-gst-kmssink-Add-support-for-xilinx-drm.patch \
>> +    file://0004-kmssink-override-stride-if-defined-in-driver.patch \
>> +    file://0005-kmssink-Fix-selection-of-source-region.patch \
>> +    file://0006-kmssink-Scale-up-to-the-screen-dimension.patch \
>> +    file://0007-kmssink-experimentation.patch \
>> +"
>
> Since these patches are being applied to a recipe that is not MACHINE
> specific, the patches should not be appended as a machine specific
> override. If they conflict with the existing source for other targets
> then it would be better to provide a additional custom version of the
> recipe as opposed to a machine specific version. Or at the least set
> the PACKAGE_ARCH to MACHINE_ARCH.
>
> Regards,
> Nathan
>
>
> This email and any attachments are intended for the sole use of the named
> recipient(s) and contain(s) confidential information that may be
> proprietary, privileged or copyrighted under applicable law. If you are not
> the intended recipient, do not read, copy, or forward this email message or
> any attachments. Delete this email message and any attachments immediately.
>