Adding dash_only and hls_only parameters #721

sr1990 · 2020-02-29T22:48:16Z

This PR is related to #651. Adding dash_only=true or hls_only=true will create DASH only or HLS only output.
Example:

$ packager \
  in=h264_baseline_360p_600.mp4,stream=audio,output=audio.mp4 \
  in=input_text.vtt,stream=text,output=output_text.vtt \
  in=h264_baseline_360p_600.mp4,stream=video,output=h264_360p.mp4, dash_only=true \
  in=h264_main_480p_1000.mp4,stream=video,output=h264_480p.mp4, hls_only=true  \
  in=h264_main_720p_3000.mp4,stream=video,output=h264_720p.mp4 \
  in=h264_high_1080p_6000.mp4,stream=video,output=h264_1080p.mp4 \
  --mpd_output h264.mpd --hls_master_playlist_output h264.m3u8

kqyang

Thanks for working on this feature! Please see my comments.

kqyang · 2020-03-02T01:02:49Z

packager/media/event/muxer_listener_factory.cc

-           CreateHlsListenersInternal(stream, stream_index, hls_notifier_)) {
-        combined_listener->AddListener(std::move(listener));
+
+    if (stream.dash_only || stream.hls_only) {


You can simplify the code to:

Suggested change

if (stream.dash_only || stream.hls_only) {

if (mpd_notifier_ && !stream.hls_only) {

...

}

if (hls_notifier_ && !stream.dash_only) {

...

}

kqyang · 2020-03-02T01:04:27Z

packager/packager.h

@@ -128,6 +128,11 @@ struct StreamDescriptor {
  std::vector<std::string> dash_accessiblities;
  /// Optional for DASH output. It defines Role elements of the stream.
  std::vector<std::string> dash_roles;
+
+  /// Optional for DASH output. Set to true if only DASH output is needed.


I'd prefer rephrasing it as:

/// Set to true to indicate that the stream is for dash only.

similar below.

kqyang · 2020-03-02T01:07:28Z

packager/app/stream_descriptor.cc

@@ -206,6 +210,14 @@ base::Optional<StreamDescriptor> ParseStreamDescriptor(
            base::SplitString(iter->second, ";", base::TRIM_WHITESPACE,
                              base::SPLIT_WANT_NONEMPTY);
        break;
+      case kDashOnlyField:
+        if (iter->second == "true")


Are you ok with using "1" or "0" to be consistent with "skip_encryption" in line 170?

We may also support all of them.

I am fine with "1" or "0". Will make the change.

kqyang · 2020-03-02T01:13:13Z

packager/packager.h

+  /// Optional for DASH output. Set to true if only DASH output is needed.
+  bool dash_only = false;
+  /// Optional for HLS output. Set to true if only HLS output is needed.
+  bool hls_only = false;
 };



Can you add a test in packager_test.py?

One option is to modify it from https://github.com/google/shaka-packager/blob/master/packager/app/test/packager_test.py#L652 and use dash_only for audio and use hls_only for video. After writing the test, you can run the below command to generate the golden files for you.

$ ninja -C out/Debug $ out/Debug/packager_test.py --test_update_golden_files=1

Will add the test. Thanks for the commands.

kqyang · 2020-03-02T01:17:50Z

packager/packager.h

+  /// Optional for DASH output. Set to true if only DASH output is needed.
+  bool dash_only = false;
+  /// Optional for HLS output. Set to true if only HLS output is needed.
+  bool hls_only = false;
 };

 class SHAKA_EXPORT Packager {


Can you also update https://github.com/google/shaka-packager/blob/master/docs/source/options/dash_options.rst and https://github.com/google/shaka-packager/blob/master/docs/source/options/hls_options.rst?

It will be great if you can add an example to https://raw.githubusercontent.com/google/shaka-packager/master/docs/source/tutorials/dash_hls_example.rst too?

Yes have updated the mentioned files. Please review and let me know.

…ntation

kqyang

Thanks. LGTM with a few more style nits.

Also, you are welcomed to add your name / your company name to https://github.com/google/shaka-packager/blob/master/AUTHORS and https://github.com/google/shaka-packager/blob/master/CONTRIBUTORS if you haven't.

kqyang · 2020-03-04T23:44:34Z

docs/source/options/dash_options.rst

+
+--dash_only=0|1
+
+    Optional. Defaults to 0 if not specified. If it is set to 1, indicates the stream is DASH only.


Max 80 characters per line. Same below.

kqyang · 2020-03-04T23:45:10Z

docs/source/tutorials/dash_hls_example.rst

+    $ packager \
+      in=h264_baseline_360p_600.mp4,stream=audio,output=audio.mp4 \
+      in=input_text.vtt,stream=text,output=output_text.vtt \
+      in=h264_baseline_360p_600.mp4,stream=video,output=h264_360p.mp4, dash_only=1 \


remove the space between "," and "dash_only" same below.

sr1990 · 2020-03-05T19:06:06Z

Hey @kqyang, do you know why Travis CI might have failed for BUILD_TYPE=Debug LIBPACKAGER_TYPE=static_library?

kqyang

Looks very good. One last comment.

As for the Travis build failure, it seems to be an issue in the build bot. I have triggered the rebuild.

kqyang · 2020-03-05T21:28:42Z

docs/source/tutorials/dash_hls_example.rst

+
+* Output DASH + HLS with dash_only and hls_only options
+
+    $ packager \


Can we update this example to use a practical use case?

For example, HLS requires WebVTT in text format while DASH supports only segmented WebVTT in mp4 format. So that is an excellent use case for the two flags.

$ packager \ 'in=h264_baseline_360p_600.mp4,stream=audio,init_segment=audio/init.mp4,segment_template=audio/$Number$.m4s' \ 'in=input_text.vtt,stream=text,init_segment=text/init.mp4,segment_template=text/$Number$.m4s',dash_only=true' \ 'in=input_text.vtt,stream=text,segment_template=text/$Number$.vtt',hls_only=true' \ 'in=h264_baseline_360p_600.mp4,stream=video,init_segment=h264_360p/init.mp4,segment_template=h264_360p/$Number$.m4s' \ 'in=h264_main_480p_1000.mp4,stream=video,init_segment=h264_480p/init.mp4,segment_template=h264_480p/$Number$.m4s' \ 'in=h264_main_720p_3000.mp4,stream=video,init_segment=h264_720p/init.mp4,segment_template=h264_720p/$Number$.m4s' \ 'in=h264_high_1080p_6000.mp4,stream=video,init_segment=h264_1080p/init.mp4,segment_template=h264_1080p/$Number$.m4s' \ --generate_static_live_mpd --mpd_output h264.mpd \ --hls_master_playlist_output h264_master.m3u8

kqyang · 2020-03-06T07:28:16Z

docs/source/tutorials/dash_hls_example.rst

+
+    $ packager \
+      'in=h264_baseline_360p_600.mp4,stream=audio,init_segment=audio/init.mp4,segment_template=audio/$Number$.m4s' \
+      'in=input_text.vtt,stream=text,init_segment=text/init.mp4,segment_template=text/$Number$.m4s',dash_only=true' \


Oops, I accidentally included an extra ' after "m4s", can you remove it? Same below.

kqyang · 2020-03-06T18:20:26Z

@sr1990 The PR has been merged. Thanks for the great work on this feature request!

Adding dash_only and hls_only parameters

30f4fc2

kqyang reviewed Mar 2, 2020

View reviewed changes

kqyang mentioned this pull request Mar 3, 2020

Package DASH and HLS subtitles. #722

Closed

Making changes as per comments, adding tests, test content and docume…

a65cb99

…ntation

kqyang reviewed Mar 4, 2020

View reviewed changes

Formatting changes

1fd9999

kqyang reviewed Mar 5, 2020

View reviewed changes

Adding a better example.

88e1a79

kqyang reviewed Mar 6, 2020

View reviewed changes

Corrections in the example.

02548a4

kqyang merged commit a1dd82d into shaka-project:master Mar 6, 2020

aradalvand mentioned this pull request May 31, 2021

Unknown field in stream descriptor ("dash_only")?! #950

Closed

github-actions bot added the status: archived Archived and locked; will not be updated label Jul 24, 2023

github-actions bot locked as resolved and limited conversation to collaborators Jul 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding dash_only and hls_only parameters #721

Adding dash_only and hls_only parameters #721

sr1990 commented Feb 29, 2020

kqyang left a comment

kqyang Mar 2, 2020

kqyang Mar 2, 2020

kqyang Mar 2, 2020

sr1990 Mar 3, 2020

kqyang Mar 2, 2020

sr1990 Mar 3, 2020

kqyang Mar 2, 2020

sr1990 Mar 3, 2020

kqyang left a comment

kqyang Mar 4, 2020

kqyang Mar 4, 2020

sr1990 commented Mar 5, 2020

kqyang left a comment

kqyang Mar 5, 2020

kqyang Mar 6, 2020

kqyang commented Mar 6, 2020

-    if (stream.dash_only || stream.hls_only) {
+    if (mpd_notifier_ && !stream.hls_only) {
+      ...
+    }
+    if (hls_notifier_ && !stream.dash_only) {
+      ...
+    }


		--dash_only=0\|1

		Optional. Defaults to 0 if not specified. If it is set to 1, indicates the stream is DASH only.


		* Output DASH + HLS with dash_only and hls_only options

		$ packager \

Adding dash_only and hls_only parameters #721

Adding dash_only and hls_only parameters #721

Conversation

sr1990 commented Feb 29, 2020

kqyang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kqyang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sr1990 commented Mar 5, 2020

kqyang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kqyang commented Mar 6, 2020