Re: [FFmpeg-devel] [PATCH 1/4] lavf/aviobuf: add ff_get_chomp_line

2018-04-12 Thread Jun Zhao 


On 2018/4/11 16:52, Marton Balint wrote:
>
>
> On Wed, 11 Apr 2018, Jun Zhao wrote:
>
>>
>>
>> On 2018/4/10 21:54, Marton Balint wrote:
>>>
>>>
>>> On Tue, 10 Apr 2018, Jun Zhao wrote:
>>>


>>>
>>> Maybe you should use ff_read_line_to_bprint instead? It already chops
>>> the trailing line endings, not any whitespace though. Generally I
>>> think we should deprecate ff_get_line, because line length limits
>>> always pop here or there...
>>>
>> ff_read_line_to_bprint usually use when we don't know the length limits
>> and need to the caller free the AVBPrint resource, in hls case, I think
>> this is a simple case, the other reason is I don't want to mix the
>> AVBPrint and c native char * in hls. Thanks.
>
> Since it's just factorization of existing code, then I guess it is OK.
Pushed , thanks the comments, Marton.
>
> But new code should not use either that or ff_get_line. (MAX_URL_SIZE
> being 4096 does not even adhere to the RFC recommendation of
> supporting at least 8000 char length URLs)
>
> https://tools.ietf.org/html/rfc7230#section-3.1.1
>
> Regards,
> Marton
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] lavf/aviobuf: add ff_get_chomp_line

2018-04-11 Thread Jun Zhao 


On 2018/4/11 16:52, Marton Balint wrote:
>
>
> On Wed, 11 Apr 2018, Jun Zhao wrote:
>
>>
>>
>> On 2018/4/10 21:54, Marton Balint wrote:
>>>
>>>
>>> On Tue, 10 Apr 2018, Jun Zhao wrote:
>>>


>>>
>>> Maybe you should use ff_read_line_to_bprint instead? It already chops
>>> the trailing line endings, not any whitespace though. Generally I
>>> think we should deprecate ff_get_line, because line length limits
>>> always pop here or there...
>>>
>> ff_read_line_to_bprint usually use when we don't know the length limits
>> and need to the caller free the AVBPrint resource, in hls case, I think
>> this is a simple case, the other reason is I don't want to mix the
>> AVBPrint and c native char * in hls. Thanks.
>
> Since it's just factorization of existing code, then I guess it is OK.
Yes
>
> But new code should not use either that or ff_get_line. (MAX_URL_SIZE
> being 4096 does not even adhere to the RFC recommendation of
> supporting at least 8000 char length URLs)
I agree this part
>
> https://tools.ietf.org/html/rfc7230#section-3.1.1
I think HLS need to fix this issue when read URL. This is the other
patchset I think.
>
> Regards,
> Marton
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] lavf/aviobuf: add ff_get_chomp_line

2018-04-11 Thread Marton Balint 



On Wed, 11 Apr 2018, Jun Zhao wrote:




On 2018/4/10 21:54, Marton Balint wrote:



On Tue, 10 Apr 2018, Jun Zhao wrote:






Maybe you should use ff_read_line_to_bprint instead? It already chops
the trailing line endings, not any whitespace though. Generally I
think we should deprecate ff_get_line, because line length limits
always pop here or there...


ff_read_line_to_bprint usually use when we don't know the length limits
and need to the caller free the AVBPrint resource, in hls case, I think
this is a simple case, the other reason is I don't want to mix the
AVBPrint and c native char * in hls. Thanks.


Since it's just factorization of existing code, then I guess it is OK.

But new code should not use either that or ff_get_line. (MAX_URL_SIZE 
being 4096 does not even adhere to the RFC recommendation of supporting at 
least 8000 char length URLs)


https://tools.ietf.org/html/rfc7230#section-3.1.1

Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] lavf/aviobuf: add ff_get_chomp_line

2018-04-10 Thread Jun Zhao 


On 2018/4/10 21:54, Marton Balint wrote:
>
>
> On Tue, 10 Apr 2018, Jun Zhao wrote:
>
>>
>>
>
> Maybe you should use ff_read_line_to_bprint instead? It already chops
> the trailing line endings, not any whitespace though. Generally I
> think we should deprecate ff_get_line, because line length limits
> always pop here or there...
>
ff_read_line_to_bprint usually use when we don't know the length limits
and need to the caller free the AVBPrint resource, in hls case, I think
this is a simple case, the other reason is I don't want to mix the
AVBPrint and c native char * in hls. Thanks.
> Regards,
> Marton
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] lavf/aviobuf: add ff_get_chomp_line

2018-04-10 Thread Marton Balint 



On Tue, 10 Apr 2018, Jun Zhao wrote:






Maybe you should use ff_read_line_to_bprint instead? It already chops the 
trailing line endings, not any whitespace though. Generally I think we 
should deprecate ff_get_line, because line length limits always pop 
here or there...


Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/4] lavf/aviobuf: add ff_get_chomp_line

2018-04-09 Thread Steven Liu 


> On 10 Apr 2018, at 08:33, Jun Zhao  wrote:
> 
> 
> <0001-lavf-aviobuf-add-ff_get_chomp_line.patch>___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

patchset LGTM

Thanks
Steven





___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 1/4] lavf/aviobuf: add ff_get_chomp_line

2018-04-09 Thread Jun Zhao 

From 58e8cb520eeeb727ee834ee81877db7c81fe089b Mon Sep 17 00:00:00 2001
From: Jun Zhao 
Date: Mon, 9 Apr 2018 23:05:42 +0800
Subject: [PATCH 1/4] lavf/aviobuf: add ff_get_chomp_line

Same as ff_get_line but strip the white-space characters in the
string tail.

Signed-off-by: Jun Zhao 
---
 libavformat/aviobuf.c  |  8 
 libavformat/internal.h | 10 ++
 2 files changed, 18 insertions(+)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 95b3364478..e752d0e1a6 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -823,6 +823,14 @@ int ff_get_line(AVIOContext *s, char *buf, int maxlen)
 return i;
 }
 
+int ff_get_chomp_line(AVIOContext *s, char *buf, int maxlen)
+{
+int len = ff_get_line(s, buf, maxlen);
+while (len > 0 && av_isspace(buf[len - 1]))
+buf[--len] = '\0';
+return len;
+}
+
 int64_t ff_read_line_to_bprint(AVIOContext *s, AVBPrint *bp)
 {
 int len, end;
diff --git a/libavformat/internal.h b/libavformat/internal.h
index c50382ad29..3582682925 100644
--- a/libavformat/internal.h
+++ b/libavformat/internal.h
@@ -299,6 +299,16 @@ void ff_put_v(AVIOContext *bc, uint64_t val);
  */
 int ff_get_line(AVIOContext *s, char *buf, int maxlen);
 
+/**
+ * Same as ff_get_line but strip the white-space characters in the text tail
+ *
+ * @param s the read-only AVIOContext
+ * @param buf buffer to store the read line
+ * @param maxlen size of the buffer
+ * @return the length of the string written in the buffer
+ */
+int ff_get_chomp_line(AVIOContext *s, char *buf, int maxlen);
+
 /**
  * Read a whole line of text from AVIOContext to an AVBPrint buffer. Stop
  * reading after reaching a \\r, a \\n, a \\r\\n, a \\0 or EOF.  The line
-- 
2.14.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel