Skip to content

Commit

Permalink
Add HttpFile implementing the HTTP PUT chunked flavor
Browse files Browse the repository at this point in the history
  • Loading branch information
amotl committed Oct 19, 2018
1 parent 344c36f commit 5f980ff
Show file tree
Hide file tree
Showing 4 changed files with 274 additions and 156 deletions.
48 changes: 36 additions & 12 deletions docs/source/tutorials/http_upload.rst
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ The produced artefacts are:
Research
--------
We identified a few sensible ways of uploading files using HTTP.
We will refer to them as "transfer modes".

1. HTTP POST

Expand Down Expand Up @@ -67,21 +68,42 @@ Implementation
This section describes the current state of the implementation,
contributions are welcome.

The "HTTP PATCH with append" (4.) method has been implemented first, but the
"HTTP PUT with chunked transfer" (3.) method suggested by @colleenkhenry
and provided by @kqyang will be implemented, being an even better variant.
As @kqyang noted, this transport mode should be more efficient than
HTTP PATCH as only one HTTP connection needs to be setup.
- The "HTTP PATCH with append" transfer mode (4.) has been implemented
and works flawlessly. However, this mode uses a full HTTP request
for each segment, so it is like medium-efficient.

All data will be declared as ``Content-Type: application/octet-stream``.
- The "HTTP PUT with chunked transfer" transfer mode (3.) suggested by
`@colleenkhenry`_ and sketched out by `@kqyang`_ is also implemented,
but segfaults after a short while. Also, while HTTP requests
are being made, no data seems to arrive at the HTTP sink.
Bummer!

As `@kqyang`_ noted, chunked transfer mode should be most efficient,
so we are definitively aiming at that target.

For pragmatic reasons, all HTTP requests will be declared as
``Content-Type: application/octet-stream``.

For controlling the transfer mode, an appropriate identifier
is prefixed to the designated url scheme, which is covered by
the HTTP/URI specification.
This saves us from introducing yet another commandline parameter.
Please refer to these examples about how to apply this to your
scenario and also refer to the example section below::

# Use "HTTP PATCH with append" transfer mode
patch.append+http://media.example.org/hls-live

# Use "HTTP PUT with chunked transfer" transfer mode
put+http://media.example.org/hls-live


Drawbacks
---------
As ``File::file_name()`` returns the real file name with its scheme prefix
(file://, http://) stripped off already by ``File::CreateInternalFile()``
and ``File::GetFileTypeInfo``, the ``http_upload`` module currently can
not account for https.
not account for https as it doesn't know about the designated protocol scheme.


Example
Expand Down Expand Up @@ -112,16 +134,15 @@ Acquire and transcode RTMP stream::
Configure and run packager::

# Define upload target
export META_PATH=http://localhost:6767/hls-live/meta
export DATA_PATH=http://localhost:6767/hls-live/media
export BASE_PATH=patch.append+http://localhost:6767/hls-live

# Go
packager \
"input=${PIPE},stream=audio,segment_template=${DATA_PATH}/bigbuckbunny-audio-aac-\$Number%04d\$.aac,playlist_name=bigbuckbunny-audio.m3u8,hls_group_id=audio" \
"input=${PIPE},stream=video,segment_template=${DATA_PATH}/bigbuckbunny-video-h264-450-\$Number%04d\$.ts,playlist_name=bigbuckbunny-video-450.m3u8" \
"input=${PIPE},stream=audio,segment_template=${BASE_PATH}/media/bigbuckbunny-audio-aac-\$Number%04d\$.aac,playlist_name=bigbuckbunny-audio.m3u8,hls_group_id=audio" \
"input=${PIPE},stream=video,segment_template=${BASE_PATH}/media/bigbuckbunny-video-h264-450-\$Number%04d\$.ts,playlist_name=bigbuckbunny-video-450.m3u8" \
--io_block_size 65536 --fragment_duration 2 --segment_duration 2 \
--time_shift_buffer_depth 3600 --preserved_segments_outside_live_window 7200 \
--hls_master_playlist_output "${META_PATH}/bigbuckbunny.m3u8" \
--hls_master_playlist_output "${BASE_PATH}/meta/bigbuckbunny.m3u8" \
--hls_playlist_type LIVE

Output
Expand Down Expand Up @@ -232,3 +253,6 @@ Have fun!
.. _httpd-reflector.py: https://gist.github.com/amotl/3ed38e461af743aeeade5a5a106c1296
.. _M3U: https://en.wikipedia.org/wiki/M3U
.. _MPEG transport stream: https://en.wikipedia.org/wiki/MPEG_transport_stream

.. _@colleenkhenry: https://github.com/colleenkhenry
.. _@kqyang: https://github.com/kqyang
14 changes: 10 additions & 4 deletions packager/file/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ const char* kCallbackFilePrefix = "callback://";
const char* kLocalFilePrefix = "file://";
const char* kMemoryFilePrefix = "memory://";
const char* kUdpFilePrefix = "udp://";
const char* kHttpFilePrefix = "http://";
const char* kHttpPutFilePrefix = "put+http://";
const char* kHttpPatchFilePrefix = "patch.append+http://";

namespace {

Expand All @@ -62,8 +63,12 @@ File* CreateCallbackFile(const char* file_name, const char* mode) {
return new CallbackFile(file_name, mode);
}

File* CreateHttpFile(const char* file_name, const char* mode) {
return new HttpFile(file_name, mode);
File* CreateHttpFilePut(const char* file_name, const char* mode) {
return new HttpFile(file_name, HttpFile::PUT_CHUNKED);
}

File* CreateHttpFilePatch(const char* file_name, const char* mode) {
return new HttpFile(file_name, HttpFile::PATCH_APPEND);
}

File* CreateLocalFile(const char* file_name, const char* mode) {
Expand Down Expand Up @@ -120,7 +125,8 @@ static const FileTypeInfo kFileTypeInfo[] = {
{kUdpFilePrefix, &CreateUdpFile, nullptr, nullptr},
{kMemoryFilePrefix, &CreateMemoryFile, &DeleteMemoryFile, nullptr},
{kCallbackFilePrefix, &CreateCallbackFile, nullptr, nullptr},
{kHttpFilePrefix, &CreateHttpFile, nullptr, nullptr},
{kHttpPutFilePrefix, &CreateHttpFilePut, nullptr, nullptr},
{kHttpPatchFilePrefix, &CreateHttpFilePatch, nullptr, nullptr},
};

base::StringPiece GetFileTypePrefix(base::StringPiece file_name) {
Expand Down
Loading

4 comments on commit 5f980ff

@rkuroiwa
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In response to shaka-project#149 (comment)
If the bug is gone, please ignore my comments.

Lines 311 and 312 of http_file.cc may be problematic for the PUT method since the memory pointed by the string pointer is probably gone when data is written.
It might make sense to make |response| a member variable and use it as a blackhole, since neither methods use the response. It should still work without setting the WRITEFUNCTION and WRITEDATA but it seems like it's not recommended
https://curl.haxx.se/mail/lib-2008-11/0271.html

I don't think 125 has to be posted to a worker pool. I don't think it hurts at the moment, but it reduces the number of variables if the problem is due to data race.

@amotl
Copy link
Collaborator Author

@amotl amotl commented on 5f980ff Oct 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear @rkuroiwa,

thanks for taking the time commenting on the code. Do you mind if i transfer your remarks to the conversation at issue shaka-project#149? Then, we can follow up on this and might make further collaborative progress by eventually getting more eyeballs on the remaining issues.

Best,
Andreas.

@rkuroiwa
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @amotl

Sure.
I thought putting comments here is better since the code is here. I was thinking discussing the design there, and the comments for the code here. Also there is a link to my comments on the issue page already (github did it for me :)).

FYI I want to setup a simple Python HTTP server and use the unit test framework to figure out the problem, just haven't gotten around to it.

Thanks,
Rintaro

@amotl
Copy link
Collaborator Author

@amotl amotl commented on 5f980ff Oct 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dear Rintaro (@rkuroiwa),

I want to setup a simple Python HTTP server [...] to figure out the problem

I prepared httpd-reflector.py for doing just that, it's part of my workbench for this job. It might also help you inspecting the HTTP requests and shows the requests for the HTTP PATCH transport flavor just fine already. See also HTTP upload » Example.

Thanks for your time,
Andreas.

Please sign in to comment.