[media] Create AudioDiscardDurationTracker #2017

osagie98 · 2023-11-29T00:32:51Z

Creates a class AudioDiscardDurationTracker to store the discard durations of audio buffers sent to pass-through audio renderers. The stored durations can then be used during media time calculation. This is implemented for the Android pass-through audio renderer.

b/313719729

codecov · 2023-11-29T01:50:40Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (def4544) 59.04% compared to head (f53060c) 58.70%.
Report is 27 commits behind head on main.

❗ Current head f53060c differs from pull request most recent head fad11a0. Consider uploading reports for the commit fad11a0 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2017      +/-   ##
==========================================
- Coverage   59.04%   58.70%   -0.34%     
==========================================
  Files        1744     1909     +165     
  Lines       80005    94125   +14120     
==========================================
+ Hits        47240    55258    +8018     
- Misses      32765    38867    +6102

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Creates a class AudioDiscardDurationTracker to store the discard durations of audio buffers sent to pass-through audio renderers. The stored durations can then be used during media time calculation. This is implemented for the Android pass-through audio renderer. b/313719729

jasonzhangxx · 2023-11-29T21:34:39Z

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

+  const AudioSampleInfo audio_sample_info = input_buffer.audio_sample_info();
+  discard_durations_by_timestamp_[input_buffer.timestamp()] =
+      audio_sample_info.discarded_duration_from_front +
+      audio_sample_info.discarded_duration_from_back;


We still need to treat discarded_duration_from_front and discarded_duration_from_back differently. When there's audio track switch, usually the last audio unit of the first stream will have non-zero discarded_duration_from_back, and the first audio unit of the second stream will have non-zero discarded_duration_from_front. Ideally, we should have only one audio gap. But if we combine discarded_duration_from_front and discarded_duration_from_back, no matter when we adjust the media time (at beginning or at end), it would introduce any extra gap.

I think we can pause the media time when it reaches input buffer's timestamp (if discarded_duration_from_front is not zero) until the media time exceeds input's timestamp + discarded_duration_from_front. And for discarded_duration_from_back, we can move or combine it to discarded_duration_from_front of next input buffer. What do you think?

I like the approach with discarded_duration_from_front. With discarded_duration_from_back, ideally I'd like to pause media time from <input timestamp + buffer duration - discarded_duration_from_back> to <input timestamp + buffer duration>, but I need to know the length of the buffer. We don't know the length of the buffer until we read the E/AC3 sync frame, however I know it's likely 32 ms (1536 samples @ 48 khz). I'm working to come up with a different solution.

I ended up hardcoding 32 ms and adding a TODO to address later.

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

starboard/android/shared/audio_renderer_passthrough.cc

This commit also adds a passthrough only option to GetSupportedAudioTestFiles(), as well as refines the AudioDiscardDurationTracker implementation.

starboard/shared/starboard/player/filter/testing/audio_discard_duration_tracker_test.cc

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.h

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.h

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

starboard/android/shared/audio_renderer_passthrough.cc

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.h

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

jasonzhangxx · 2024-02-07T22:41:36Z

starboard/android/shared/audio_renderer_passthrough.cc

+  // AC-3/EAC-3 sync frames with 1536 samples at 48khz have a duration of 32 ms.
+  // TODO: Determine this value at runtime.
+  const SbTime kBufferDuration = 32 * kSbTimeMillisecond;
+  discard_duration_tracker_.CacheMultipleDiscardDurations(input_buffers,


Can we move it to AudioRendererPassthrough::OnDecoderOutput()? And maybe we can reuse |frames_per_input_buffer_|.

That would be ideal, however the AudioSampleInfo would need to be copied to the DecodedAudio for the discard information. I wanted to avoid adding that to the DecodedAudio interface for just that use case. What do you think?

Or rather, just the discard durations would need to be stored.

jasonzhangxx · 2024-02-07T22:42:27Z

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

@@ -0,0 +1,107 @@
+// Copyright 2023 The Cobalt Authors. All Rights Reserved.


nit: Now it's 2024.

jasonzhangxx · 2024-02-07T22:44:57Z

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

+      input_buffer->audio_sample_info().discarded_duration_from_back;
+
+  if (discard_duration_from_front + discard_duration_from_back >=
+      buffer_duration) {


Can we move this check to AudioRendererPassthrough::OnDecoderOutput() and discard the whole buffer if discarded duration is larger than the buffer duration? We can then add a SB_DCHECK() here to ensure buffer duration is larger than discarded duration.

jasonzhangxx · 2024-02-07T22:47:37Z

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

+  // As a lot of time may have passed since the last call to
+  // AdjustTimeForTotalDiscardDuration(), remove all AudioDiscardInfos that
+  // have already been passed by the |timestamp|.
+  while (discard_infos_.size() > 0 &&


nit: Usually, we prefer !discard_infos_.empty().

jasonzhangxx · 2024-02-07T22:49:40Z

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

+    SbTime discard_end_timestamp =
+        discard_start_timestamp + discard_infos_.front().discard_duration;
+    if (timestamp >= discard_start_timestamp &&
+        timestamp < discard_end_timestamp) {


timestamp < discard_end_timestamp here is redundant. It's guaranteed by the while loop above.

jasonzhangxx · 2024-02-07T23:08:01Z

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.cc

+      << ". AudioDiscardDurationTracker::AdjustTimeForTotalDiscardDuration() "
+         "requires monotonically increasing timestamps.";
+  last_received_timestamp_ = timestamp;
+


discard_start_timestamp is based on adjusted timestamp, but the input timestamp is not adjusted. It should be adjusted before comparing to the discard_start_timestamp. And the return value is discard_start_timestamp - total_discard_duration_, that adjusts the timestamp on an adjusted timestamp.

For example, if there're two inputs and both have 20ms audio and discarded duration from front of 5ms, the total played time would be 40ms(wall time), and discard_infos_ would be {0ms, 5ms}, {15ms, 5ms}. During 0-15ms (wall time), the returned value works as expected. During 15-20ms(wall time), the returned value would be 10ms, which is not right. It's expected to freeze the timestamp at 20-25ms(wall time).

osagie98 force-pushed the address-passthrough-partial-audio branch 2 times, most recently from 900559f to 8880943 Compare November 29, 2023 20:38

osagie98 force-pushed the address-passthrough-partial-audio branch from 8880943 to ddb75db Compare November 29, 2023 20:40

osagie98 marked this pull request as ready for review November 29, 2023 20:45

osagie98 requested review from sideb0ard, jasonzhangxx, xiaomings and borongc November 29, 2023 20:46

jasonzhangxx reviewed Nov 29, 2023

View reviewed changes

osagie98 added 4 commits November 30, 2023 10:11

Only modify media time once frames are played

20994e8

Add AudioDiscardDurationTracker tests

301cf2c

This commit also adds a passthrough only option to GetSupportedAudioTestFiles(), as well as refines the AudioDiscardDurationTracker implementation.

Filter tests from unsupported platforms

c0f22c8

Allow uninstantiated parameterized test

84508a5

xiaomings reviewed Dec 8, 2023

View reviewed changes

starboard/shared/starboard/player/filter/audio_discard_duration_tracker.h Show resolved Hide resolved

Add mutex to AudioDiscardDurationTracker

37974ca

osagie98 requested review from jasonzhangxx and xiaomings December 11, 2023 23:11

xiaomings reviewed Dec 12, 2023

View reviewed changes

osagie98 added 2 commits December 12, 2023 14:55

Make CacheDiscardDuration Private

8615eaa

Check for mutex acquisition

f53060c

osagie98 requested a review from xiaomings December 12, 2023 23:14

jasonzhangxx reviewed Feb 7, 2024

View reviewed changes

osagie98 added 3 commits February 9, 2024 22:04

Merge branch 'main' into address-passthrough-partial-audio

7b8c969

Remove use of SbTime

5119001

Fix double time adjustment issue

fad11a0

osagie98 mentioned this pull request Mar 13, 2024

[media] Adjust partial audio timestamps in tests #2583

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[media] Create AudioDiscardDurationTracker #2017

[media] Create AudioDiscardDurationTracker #2017

osagie98 commented Nov 29, 2023 •

edited by jasonzhangxx

Loading

codecov bot commented Nov 29, 2023 •

edited

Loading

jasonzhangxx Nov 29, 2023

jasonzhangxx Nov 29, 2023

osagie98 Nov 30, 2023 •

edited

Loading

osagie98 Nov 30, 2023

jasonzhangxx Feb 7, 2024

osagie98 Feb 12, 2024

osagie98 Feb 12, 2024

jasonzhangxx Feb 7, 2024

jasonzhangxx Feb 7, 2024

jasonzhangxx Feb 7, 2024

jasonzhangxx Feb 7, 2024

jasonzhangxx Feb 7, 2024

		@@ -0,0 +1,107 @@
		// Copyright 2023 The Cobalt Authors. All Rights Reserved.

[media] Create AudioDiscardDurationTracker #2017

Are you sure you want to change the base?

[media] Create AudioDiscardDurationTracker #2017

Conversation

osagie98 commented Nov 29, 2023 • edited by jasonzhangxx Loading

codecov bot commented Nov 29, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

osagie98 Nov 30, 2023 • edited Loading

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

Choose a reason for hiding this comment

osagie98 commented Nov 29, 2023 •

edited by jasonzhangxx

Loading

codecov bot commented Nov 29, 2023 •

edited

Loading

osagie98 Nov 30, 2023 •

edited

Loading