diff mbox series

[FFmpeg-devel] Bugfix for #9135

Message ID a3b14522-a29e-31a7-0c11-6d0c72a84718@posteo.net
State New
Headers show
Series [FFmpeg-devel] Bugfix for #9135 | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

Peter White March 2, 2021, 4:03 p.m. UTC
Hello dear ffmpeg developers,

I have just created ticket #9135 on the bug tracker. According to the 
instructions, I am sending the patch to this list, see attachment.

Peter

Comments

Peter White March 2, 2021, 4:25 p.m. UTC | #1
Sorry, apparently patches need to be sent inline, I just noticed. Please 
excuse my ignorance.

Peter


diff --git a/configure b/configure
index d11942fced..bcc845668b 100755
--- a/configure
+++ b/configure
@@ -6490,7 +6490,7 @@ enabled mmal              && { check_lib mmal 
interface/mmal/mmal.h mmal_port_co
                                 die "ERROR: mmal not found" &&
                                 check_func_headers 
interface/mmal/mmal.h "MMAL_PARAMETER_VIDEO_MAX_NUM_CALLBACKS"; }
  enabled openal            && { { for al_extralibs in "${OPENAL_LIBS}" 
"-lopenal" "-lOpenAL32"; do
-                               check_lib openal 'AL/al.h' alGetError 
"${al_extralibs}" && break; done } ||
+                               check_lib openal 'AL/al.h' alGetError 
"${al_extralibs}" && break; done; } ||
                                 die "ERROR: openal not found"; } &&
                               { test_cpp_condition "AL/al.h" 
"defined(AL_VERSION_1_1)" ||
                                 die "ERROR: openal must be installed 
and version must be 1.1 or compatible"; }
James Almer March 2, 2021, 4:56 p.m. UTC | #2
On 3/2/2021 1:25 PM, Peter White wrote:
> Sorry, apparently patches need to be sent inline, I just noticed. Please 
> excuse my ignorance.

They don't need to be inline, although it is preferred. What they need 
to be is made with git format-patch, which preserves authorship 
information and commit message.

Also, the commit message should be descriptive of what it fixes, and not 
only reference a ticket number.

> 
> Peter
> 
> 
> diff --git a/configure b/configure
> index d11942fced..bcc845668b 100755
> --- a/configure
> +++ b/configure
> @@ -6490,7 +6490,7 @@ enabled mmal              && { check_lib mmal 
> interface/mmal/mmal.h mmal_port_co
>                                  die "ERROR: mmal not found" &&
>                                  check_func_headers 
> interface/mmal/mmal.h "MMAL_PARAMETER_VIDEO_MAX_NUM_CALLBACKS"; }
>   enabled openal            && { { for al_extralibs in "${OPENAL_LIBS}" 
> "-lopenal" "-lOpenAL32"; do
> -                               check_lib openal 'AL/al.h' alGetError 
> "${al_extralibs}" && break; done } ||
> +                               check_lib openal 'AL/al.h' alGetError 
> "${al_extralibs}" && break; done; } ||
>                                  die "ERROR: openal not found"; } &&
>                                { test_cpp_condition "AL/al.h" 
> "defined(AL_VERSION_1_1)" ||
>                                  die "ERROR: openal must be installed 
> and version must be 1.1 or compatible"; }
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
Peter White March 2, 2021, 5:36 p.m. UTC | #3
On 02.03.21 17:56, James Almer wrote:
> On 3/2/2021 1:25 PM, Peter White wrote:
>> Sorry, apparently patches need to be sent inline, I just noticed. 
>> Please excuse my ignorance.
> 
> They don't need to be inline, although it is preferred. What they need 
> to be is made with git format-patch, which preserves authorship 
> information and commit message.

Thanks for your explanation. Hopefully now it is OK.

> Also, the commit message should be descriptive of what it fixes, and not 
> only reference a ticket number.

Like this?


 From 771ac6d3395f84d0969b088d2d0680281c1062d3 Mon Sep 17 00:00:00 2001
From: Peter White <peter.white@posteo.net>
Date: Tue, 2 Mar 2021 18:10:49 +0100
Subject: [PATCH] configure: Fix bashism in openal check.

Apparently zsh, when run in sh-compatibility mode (started as 'sh') is
very strict about command lists, which must always end with a semicolon.
Even dash let this one slide.

Fixes #9135
---
  configure | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index d11942fced..bcc845668b 100755
--- a/configure
+++ b/configure
@@ -6490,7 +6490,7 @@ enabled mmal              && { check_lib mmal 
interface/mmal/mmal.h mmal_port_co
                                 die "ERROR: mmal not found" &&
                                 check_func_headers 
interface/mmal/mmal.h "MMAL_PARAMETER_VIDEO_MAX_NUM_CALLBACKS"; }
  enabled openal            && { { for al_extralibs in "${OPENAL_LIBS}" 
"-lopenal" "-lOpenAL32"; do
-                               check_lib openal 'AL/al.h' alGetError 
"${al_extralibs}" && break; done } ||
+                               check_lib openal 'AL/al.h' alGetError 
"${al_extralibs}" && break; done; } ||
                                 die "ERROR: openal not found"; } &&
                               { test_cpp_condition "AL/al.h" 
"defined(AL_VERSION_1_1)" ||
                                 die "ERROR: openal must be installed 
and version must be 1.1 or compatible"; }
Marton Balint March 2, 2021, 10:15 p.m. UTC | #4
On Tue, 2 Mar 2021, Peter White wrote:

> On 02.03.21 17:56, James Almer wrote:
>> On 3/2/2021 1:25 PM, Peter White wrote:
>>> Sorry, apparently patches need to be sent inline, I just noticed. 
>>> Please excuse my ignorance.
>> 
>> They don't need to be inline, although it is preferred. What they need 
>> to be is made with git format-patch, which preserves authorship 
>> information and commit message.
>
> Thanks for your explanation. Hopefully now it is OK.
>
>> Also, the commit message should be descriptive of what it fixes, and not 
>> only reference a ticket number.
>
> Like this?

Well, yes and no. If you want to send inline patches, then only the patch 
as generated by git format-patch should be your mail, and no line wrapping 
should occur because that corrupts the patch. Copy-pasting the patch to a 
regular mail client does not work (a regular mail client wraps lines, eats 
up whitespace, etc), so I think most people just use git send-email.

If you can't or won't use that, then sending the patch as attachment is 
the safer bet to avoid patch corruption. Many people does that, even if 
that is not what we generally prefer.

Regards,
Marton
Peter White March 2, 2021, 11 p.m. UTC | #5
On 02.03.21 23:15, Marton Balint wrote:
> 
> 
> On Tue, 2 Mar 2021, Peter White wrote:
> 
>> On 02.03.21 17:56, James Almer wrote:
>>> On 3/2/2021 1:25 PM, Peter White wrote:
>>>> Sorry, apparently patches need to be sent inline, I just noticed. 
>>>> Please excuse my ignorance.
>>>
>>> They don't need to be inline, although it is preferred. What they 
>>> need to be is made with git format-patch, which preserves authorship 
>>> information and commit message.
>>
>> Thanks for your explanation. Hopefully now it is OK.
>>
>>> Also, the commit message should be descriptive of what it fixes, and 
>>> not only reference a ticket number.
>>
>> Like this?
> 
> Well, yes and no. If you want to send inline patches, then only the 
> patch as generated by git format-patch should be your mail, and no line 
> wrapping should occur because that corrupts the patch. Copy-pasting the 
> patch to a regular mail client does not work (a regular mail client 
> wraps lines, eats up whitespace, etc), so I think most people just use 
> git send-email.

Oh, it looks like Thunderbird is really stupid in that regard. In my 
"Sent" folder the patch looks correct. Not so on the list. :(

> If you can't or won't use that, then sending the patch as attachment is 
> the safer bet to avoid patch corruption. Many people does that, even if 
> that is not what we generally prefer.

I have never come around to setting up postfix for forwarding mails 
externally on my current system. So, for now, I am using the second best 
solution. Please see the attachment.

Thanks again for your explanation.

Peter
Reimar Döffinger March 3, 2021, 5:17 p.m. UTC | #6
> 
> I have never come around to setting up postfix for forwarding mails externally on my current system. So, for now, I am using the second best solution. Please see the attachment.

Maybe not worth the effort, but FYI git can communicate directly with
a SMTP server nowadays, and msmtmp is a simpler solution if all
you want is a simple forwarding sendmail (not normally
needed for git anymore though).
Peter White March 5, 2021, 10:59 a.m. UTC | #7
This is a test to check if my postfix setup does what I intend.
Peter White March 5, 2021, 1:43 p.m. UTC | #8
On Wed, Mar 03, 2021 at 06:17:36PM +0100, Reimar Döffinger wrote:
> > 
> > I have never come around to setting up postfix for forwarding mails
> > externally on my current system. So, for now, I am using the second
> > best solution. Please see the attachment.
> 
> Maybe not worth the effort, but FYI git can communicate directly with
> a SMTP server nowadays, and msmtmp is a simpler solution if all
> you want is a simple forwarding sendmail (not normally
> needed for git anymore though).

Thank you for the information. I always meant to set up postfix properly
anyways but procrastinated it until now. Now I have finally done it.

Shall I send the patch again, opening a new thread, or will the previous
attachment suffice? So far it seems to be getting ignored, or do I
simply need to be more patient?


Peter
Carl Eugen Hoyos March 6, 2021, 12:23 p.m. UTC | #9
Am Mi., 3. März 2021 um 00:01 Uhr schrieb Peter White <peter.white@posteo.net>:

> Please see the attachment.

I'll push this if there are no objections.

Carl Eugen
diff mbox series

Patch

diff --git a/configure b/configure
index d11942fced..bcc845668b 100755
--- a/configure
+++ b/configure
@@ -6490,7 +6490,7 @@  enabled mmal              && { check_lib mmal interface/mmal/mmal.h mmal_port_co
                                die "ERROR: mmal not found" &&
                                check_func_headers interface/mmal/mmal.h "MMAL_PARAMETER_VIDEO_MAX_NUM_CALLBACKS"; }
 enabled openal            && { { for al_extralibs in "${OPENAL_LIBS}" "-lopenal" "-lOpenAL32"; do
-                               check_lib openal 'AL/al.h' alGetError "${al_extralibs}" && break; done } ||
+                               check_lib openal 'AL/al.h' alGetError "${al_extralibs}" && break; done; } ||
                                die "ERROR: openal not found"; } &&
                              { test_cpp_condition "AL/al.h" "defined(AL_VERSION_1_1)" ||
                                die "ERROR: openal must be installed and version must be 1.1 or compatible"; }