diff mbox series

[FFmpeg-devel] avformat/http: Add short_seek_size option

Message ID 20211115155242.1722587-1-derek.buitenhuis@gmail.com
State New
Headers show
Series [FFmpeg-devel] avformat/http: Add short_seek_size option | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Derek Buitenhuis Nov. 15, 2021, 3:52 p.m. UTC
In 45bfe8b838275235412777dd430206d9a24eb3ee, short_seek_threshold was removed
from the public AVIO struct. Although this option was private and not intended
to be used by public API users, it was nonetheless, because it provided functionality
that could otherwise not be gained via public API.

This was especially important for networked I/O like HTTP, where the internal
size for lavf could be way to small depending on the specifics of a user's
usecase, such as reading interlavd media files from cloud storage.

Add an AVOption to make this functionality accessible to the HTTP client.

Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
---
Same as last version but I'm marking it as non-RFC. I think I prefer the AVOption approach.

Comments welcome.
---
 libavformat/http.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

James Almer Nov. 15, 2021, 7:53 p.m. UTC | #1
On 11/15/2021 12:52 PM, Derek Buitenhuis wrote:
> In 45bfe8b838275235412777dd430206d9a24eb3ee, short_seek_threshold was removed
> from the public AVIO struct. Although this option was private and not intended
> to be used by public API users, it was nonetheless, because it provided functionality
> that could otherwise not be gained via public API.
> 
> This was especially important for networked I/O like HTTP, where the internal
> size for lavf could be way to small depending on the specifics of a user's
> usecase, such as reading interlavd media files from cloud storage.
> 
> Add an AVOption to make this functionality accessible to the HTTP client.
> 
> Signed-off-by: Derek Buitenhuis <derek.buitenhuis@gmail.com>
> ---
> Same as last version but I'm marking it as non-RFC. I think I prefer the AVOption approach.
> 
> Comments welcome.

Don't forget to bump micro before you push.

> ---
>   libavformat/http.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/libavformat/http.c b/libavformat/http.c
> index 476b9a8456..0dc1ce0f43 100644
> --- a/libavformat/http.c
> +++ b/libavformat/http.c
> @@ -126,6 +126,7 @@ typedef struct HTTPContext {
>       int is_multi_client;
>       HandshakeState handshake_step;
>       int is_connected_server;
> +    int short_seek_size;
>   } HTTPContext;
>   
>   #define OFFSET(x) offsetof(HTTPContext, x)
> @@ -167,6 +168,7 @@ static const AVOption options[] = {
>       { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 2, D | E },
>       { "resource", "The resource requested by a client", OFFSET(resource), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
>       { "reply_code", "The http status code to return to a client", OFFSET(reply_code), AV_OPT_TYPE_INT, { .i64 = 200}, INT_MIN, 599, E},
> +    { "short_seek_size", "Threshold to favor readahead over seek.", OFFSET(short_seek_size), AV_OPT_TYPE_INT, { .i64 = -1 }, 1, INT64_MAX, D },

-1 default when the valid range is 1 to INT64_MAX?

>       { NULL }
>   };
>   
> @@ -1842,6 +1844,8 @@ static int http_get_file_handle(URLContext *h)
>   static int http_get_short_seek(URLContext *h)
>   {
>       HTTPContext *s = h->priv_data;
> +    if (s->short_seek_size >= 1)
> +        return s->short_seek_size;
>       return ffurl_get_short_seek(s->hd);
>   }
>   
>
Derek Buitenhuis Nov. 15, 2021, 9:13 p.m. UTC | #2
On 11/15/2021 7:53 PM, James Almer wrote:
> Don't forget to bump micro before you push.

Yep.

>> +    { "short_seek_size", "Threshold to favor readahead over seek.", OFFSET(short_seek_size), AV_OPT_TYPE_INT, { .i64 = -1 }, 1, INT64_MAX, D },
> 
> -1 default when the valid range is 1 to INT64_MAX?

D'oh. I had an uncommited change making it 0, and 0 to INT64_MAX.

- Derek
James Almer Nov. 15, 2021, 10:49 p.m. UTC | #3
On 11/15/2021 6:13 PM, Derek Buitenhuis wrote:
> On 11/15/2021 7:53 PM, James Almer wrote:
>> Don't forget to bump micro before you push.
> 
> Yep.
> 
>>> +    { "short_seek_size", "Threshold to favor readahead over seek.", OFFSET(short_seek_size), AV_OPT_TYPE_INT, { .i64 = -1 }, 1, INT64_MAX, D },
>>
>> -1 default when the valid range is 1 to INT64_MAX?
> 
> D'oh. I had an uncommited change making it 0, and 0 to INT64_MAX.

Make it INT_MAX while at it. The field is an int.

> 
> - Derek
> _______________________________________________
> 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".
>
diff mbox series

Patch

diff --git a/libavformat/http.c b/libavformat/http.c
index 476b9a8456..0dc1ce0f43 100644
--- a/libavformat/http.c
+++ b/libavformat/http.c
@@ -126,6 +126,7 @@  typedef struct HTTPContext {
     int is_multi_client;
     HandshakeState handshake_step;
     int is_connected_server;
+    int short_seek_size;
 } HTTPContext;
 
 #define OFFSET(x) offsetof(HTTPContext, x)
@@ -167,6 +168,7 @@  static const AVOption options[] = {
     { "listen", "listen on HTTP", OFFSET(listen), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 2, D | E },
     { "resource", "The resource requested by a client", OFFSET(resource), AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, E },
     { "reply_code", "The http status code to return to a client", OFFSET(reply_code), AV_OPT_TYPE_INT, { .i64 = 200}, INT_MIN, 599, E},
+    { "short_seek_size", "Threshold to favor readahead over seek.", OFFSET(short_seek_size), AV_OPT_TYPE_INT, { .i64 = -1 }, 1, INT64_MAX, D },
     { NULL }
 };
 
@@ -1842,6 +1844,8 @@  static int http_get_file_handle(URLContext *h)
 static int http_get_short_seek(URLContext *h)
 {
     HTTPContext *s = h->priv_data;
+    if (s->short_seek_size >= 1)
+        return s->short_seek_size;
     return ffurl_get_short_seek(s->hd);
 }