[FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an extra space in the original comment

[FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an extra space in the original comment

[Jack Lau via ffmpeg-devel]

---
 libavformat/hls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 3bdc1bc848..c2130bb883 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1993,7 +1993,7 @@ static int hls_read_header(AVFormatContext *s)
         return ret;
 
     /* XXX: Some HLS servers don't like being sent the range header,
-       in this case, need to  setting http_seekable = 0 to disable
+       in this case, need to setting http_seekable = 0 to disable
        the range header */
     av_dict_set_int(&c->avio_opts, "seekable", c->http_seekable, 0);
 
-- 
2.48.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an extra space in the original comment

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Jack Lau via ffmpeg-devel
> Sent: Monday, February 10, 2025 1:34 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Jack Lau <jacklau1222@qq.com>
> Subject: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an
> extra space in the original comment
> 
> ---
>  libavformat/hls.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 3bdc1bc848..c2130bb883 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -1993,7 +1993,7 @@ static int hls_read_header(AVFormatContext *s)
>          return ret;
> 
>      /* XXX: Some HLS servers don't like being sent the range header,
> -       in this case, need to  setting http_seekable = 0 to disable
> +       in this case, need to setting http_seekable = 0 to disable
>         the range header */
>      av_dict_set_int(&c->avio_opts, "seekable", c->http_seekable, 0);
> 
> --
> 2.48.1

Hi Jack,

you're not working on a proof of concept regarding the vulnerability of the GA, right? 
(just kidding)

As an idea, you might be able to give that patch a little bit more meaning by also fixing the grammar.

Best wishes
sw
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an extra space in the original comment

[Jack Lau]

Hi,

Thanks for your reply.

I'm trying to fix some issues with hls. I happened to see this typo. Since I saw in the ffmpeg documentation that cosmetic changes should be kept in separate patches, so i submitted it first. 

Thank you for your advice, I will try my best to submit more important patches.

Best wishes
Jack


> On Feb 10, 2025, at 08:54, Soft Works <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of
>> Jack Lau via ffmpeg-devel
>> Sent: Monday, February 10, 2025 1:34 AM
>> To: ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
>> Cc: Jack Lau <jacklau1222@qq.com <mailto:jacklau1222@qq.com>>
>> Subject: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an
>> extra space in the original comment
>> 
>> ---
>> libavformat/hls.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>> index 3bdc1bc848..c2130bb883 100644
>> --- a/libavformat/hls.c
>> +++ b/libavformat/hls.c
>> @@ -1993,7 +1993,7 @@ static int hls_read_header(AVFormatContext *s)
>>         return ret;
>> 
>>     /* XXX: Some HLS servers don't like being sent the range header,
>> -       in this case, need to  setting http_seekable = 0 to disable
>> +       in this case, need to setting http_seekable = 0 to disable
>>        the range header */
>>     av_dict_set_int(&c->avio_opts, "seekable", c->http_seekable, 0);
>> 
>> --
>> 2.48.1
> 
> Hi Jack,
> 
> you're not working on a proof of concept regarding the vulnerability of the GA, right? 
> (just kidding)
> 
> As an idea, you might be able to give that patch a little bit more meaning by also fixing the grammar.
> 
> Best wishes
> sw
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an extra space in the original comment

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Jack Lau
> Sent: Monday, February 10, 2025 2:13 AM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is
> an extra space in the original comment
> 
> > On Feb 10, 2025, at 08:54, Soft Works <softworkz-at-
> hotmail.com@ffmpeg.org> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org
> <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of
> >> Jack Lau via ffmpeg-devel
> >> Sent: Monday, February 10, 2025 1:34 AM
> >> To: ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> >> Cc: Jack Lau <jacklau1222@qq.com <mailto:jacklau1222@qq.com>>
> >> Subject: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an
> >> extra space in the original comment
> >>
> >> ---
> >> libavformat/hls.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/hls.c b/libavformat/hls.c
> >> index 3bdc1bc848..c2130bb883 100644
> >> --- a/libavformat/hls.c
> >> +++ b/libavformat/hls.c
> >> @@ -1993,7 +1993,7 @@ static int hls_read_header(AVFormatContext
> *s)
> >>         return ret;
> >>
> >>     /* XXX: Some HLS servers don't like being sent the range
> header,
> >> -       in this case, need to  setting http_seekable = 0 to
> disable
> >> +       in this case, need to setting http_seekable = 0 to disable
> >>        the range header */
> >>     av_dict_set_int(&c->avio_opts, "seekable", c->http_seekable,
> 0);
> >>
> >> --
> >> 2.48.1
> >
> > Hi Jack,
> >
> > you're not working on a proof of concept regarding the
> vulnerability of the GA, right?
> > (just kidding)
> >
> > As an idea, you might be able to give that patch a little bit more
> meaning by also fixing the grammar.
> >
> > Best wishes
> > sw
> > _______________________________________________


> 
> Hi,
> 
> Thanks for your reply.
> 
> I'm trying to fix some issues with hls. I happened to see this typo.
> Since I saw in the ffmpeg documentation that cosmetic changes should
> be kept in separate patches, so i submitted it first.
> 
> Thank you for your advice, I will try my best to submit more
> important patches.
> 
> Best wishes
> Jack

Hi,

please do not top-post (ask AI if you don't know what it is). It's a rule here, probably because in those plaintext messages without formatting it's hard to follow when some are replying at the top and some at the bottom.

Cosmetic changes should be in a separate commit but they can be in the same patchset.

Personally, I don't think it's unimportant to fix whitespace, spelling and formatting issues. There's value in everything that improves the code, but you also need to consider efficiency and think about the time that gets bound for others dealing with a single-char non-functional change. 
If I would make such commit(s), then I would go through a large number of code files (like all from a lib) looking for similar issues and include all of them in my patch, so that a reviewer/maintainer sees that I have really spent effort on it, and they feel that the given value is worth their time.

Best
sw



_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an extra space in the original comment

[Jack Lau]

> On Feb 10, 2025, at 09:36, Soft Works <softworkz-at-hotmail.com@ffmpeg.org> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of
>> Jack Lau
>> Sent: Monday, February 10, 2025 2:13 AM
>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel@ffmpeg.org <mailto:devel@ffmpeg.org>>
>> Subject: Re: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is
>> an extra space in the original comment
>> 
>>> On Feb 10, 2025, at 08:54, Soft Works <softworkz-at-
>> hotmail.com@ffmpeg.org <mailto:hotmail.com@ffmpeg.org>> wrote:
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org <mailto:ffmpeg-devel-bounces@ffmpeg.org>
>> <mailto:ffmpeg-devel-bounces@ffmpeg.org>> On Behalf Of
>>>> Jack Lau via ffmpeg-devel
>>>> Sent: Monday, February 10, 2025 1:34 AM
>>>> To: ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> <mailto:ffmpeg-devel@ffmpeg.org>
>>>> Cc: Jack Lau <jacklau1222@qq.com <mailto:jacklau1222@qq.com> <mailto:jacklau1222@qq.com>>
>>>> Subject: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an
>>>> extra space in the original comment
>>>> 
>>>> ---
>>>> libavformat/hls.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> 
>>>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>>>> index 3bdc1bc848..c2130bb883 100644
>>>> --- a/libavformat/hls.c
>>>> +++ b/libavformat/hls.c
>>>> @@ -1993,7 +1993,7 @@ static int hls_read_header(AVFormatContext
>> *s)
>>>>        return ret;
>>>> 
>>>>    /* XXX: Some HLS servers don't like being sent the range
>> header,
>>>> -       in this case, need to  setting http_seekable = 0 to
>> disable
>>>> +       in this case, need to setting http_seekable = 0 to disable
>>>>       the range header */
>>>>    av_dict_set_int(&c->avio_opts, "seekable", c->http_seekable,
>> 0);
>>>> 
>>>> --
>>>> 2.48.1
>>> 
>>> Hi Jack,
>>> 
>>> you're not working on a proof of concept regarding the
>> vulnerability of the GA, right?
>>> (just kidding)
>>> 
>>> As an idea, you might be able to give that patch a little bit more
>> meaning by also fixing the grammar.
>>> 
>>> Best wishes
>>> sw
>>> _______________________________________________
> 
> 
>> 
>> Hi,
>> 
>> Thanks for your reply.
>> 
>> I'm trying to fix some issues with hls. I happened to see this typo.
>> Since I saw in the ffmpeg documentation that cosmetic changes should
>> be kept in separate patches, so i submitted it first.
>> 
>> Thank you for your advice, I will try my best to submit more
>> important patches.
>> 
>> Best wishes
>> Jack
> 
> Hi,
> 
> please do not top-post (ask AI if you don't know what it is). It's a rule here, probably because in those plaintext messages without formatting it's hard to follow when some are replying at the top and some at the bottom.
> 
> Cosmetic changes should be in a separate commit but they can be in the same patchset.
> 
> Personally, I don't think it's unimportant to fix whitespace, spelling and formatting issues. There's value in everything that improves the code, but you also need to consider efficiency and think about the time that gets bound for others dealing with a single-char non-functional change. 
> If I would make such commit(s), then I would go through a large number of code files (like all from a lib) looking for similar issues and include all of them in my patch, so that a reviewer/maintainer sees that I have really spent effort on it, and they feel that the given value is worth their time.
> 
> Best
> sw
> 
> 
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe”.


Hi,

Thank you very much for taking the time to point out my mistakes and for sharing your perspective—I completely agree.

I will make sure not to encounter these issues again in the future.

Thanks again!

Jack


_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an extra space in the original comment

[Marth64]

I don't think its fair to shoot this down, its a simple but valid tidy up work.
I find typos and such when browsing code distracting and readability
important down the road.
Not everyone's first language is English and grammar correction may
not come instinctively.

Thank you for the contribution Jack. Unless there is any objection on
grounds of maintaining commit history, I'll adjust the grammar of
"need to setting" -> "we need to set" and push this soon.
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH] avformat/hls: fix typo There is an extra space in the original comment

[Jack Lau]

> On Feb 10, 2025, at 11:08, Marth64 <marth64@proxyid.net> wrote:
> 
> I don't think its fair to shoot this down, its a simple but valid tidy up work.
> I find typos and such when browsing code distracting and readability
> important down the road.
> Not everyone's first language is English and grammar correction may
> not come instinctively.
> 
> Thank you for the contribution Jack. Unless there is any objection on
> grounds of maintaining commit history, I'll adjust the grammar of
> "need to setting" -> "we need to set" and push this soon.
> _______________________________________________
> 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”.

Hi Marth,

Thanks very much for your reply. I feel the warmth of the open-source community. I indeed did not notice this grammar issue, and I will be more careful next time.

It's hard to express my gratitude in words.🥹

Best regards,
Jack
_______________________________________________
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".