Skip to content

Commit d35966d

Browse files
ChunMinChangChunMinChang
committed
Bug 1978232 - Simplify MediaDataEncoder gTests and improve error handling r=jolin
This patch refactors `MediaDataEncoder` gTests to simplify test utility logic and improve error handling. 1. Simplify utility functions like `Encode()` and `Drain()` by directly checking the `ResolveOrRejectValue` returned from `WaitFor` instead of passing variables through `media::Await`'s callbacks and then check their values. 2. Add helper utilities to convert a `ResolveOrRejectValue` into a `Result`, allowing the use of `MOZ_TRY` for cleaner error handling. 3. Return errors directly from test utilities rather than returning an incomplete encoded outputs. This makes debugging easier, since tests now fail with the actual error instead of falling back to a length mismatch failure. Differential Revision: https://phabricator.services.mozilla.com/D262211
1 parent eb6516a commit d35966d

File tree

3 files changed

+71
-74
lines changed

3 files changed

+71
-74
lines changed

‎dom/media/gtest/TestMediaDataEncoder.cpp‎

Lines changed: 65 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "mozilla/AbstractThread.h"
1818
#include "mozilla/Preferences.h"
1919
#include "mozilla/SpinEventLoopUntil.h"
20+
#include "mozilla/gtest/WaitFor.h"
2021
#include "mozilla/media/MediaUtils.h" // For media::Await
2122

2223
#ifdef MOZ_WIDGET_ANDROID
@@ -50,6 +51,16 @@
5051
} \
5152
} while (0)
5253

54+
#define GET_OR_RETURN_ON_ERROR(expr) \
55+
__extension__({ \
56+
auto mozTryVarTempResult = ::mozilla::ToResult(expr); \
57+
if (MOZ_UNLIKELY(mozTryVarTempResult.isErr())) { \
58+
EXPECT_TRUE(false); \
59+
return; \
60+
} \
61+
mozTryVarTempResult.unwrap(); \
62+
})
63+
5364
#define BLOCK_SIZE 64
5465
#define NUM_FRAMES 150UL
5566
#define FRAME_RATE 30
@@ -219,13 +230,8 @@ static bool EnsureInit(const RefPtr<MediaDataEncoder>& aEncoder) {
219230
if (!aEncoder) {
220231
return false;
221232
}
222-
223-
bool succeeded;
224-
media::Await(
225-
GetMediaThreadPool(MediaThreadType::SUPERVISOR), aEncoder->Init(),
226-
[&succeeded](bool) { succeeded = true; },
227-
[&succeeded](const MediaResult& r) { succeeded = false; });
228-
return succeeded;
233+
auto r = WaitFor(aEncoder->Init());
234+
return r.isOk();
229235
}
230236

231237
void WaitForShutdown(const RefPtr<MediaDataEncoder>& aEncoder) {
@@ -245,51 +251,33 @@ void WaitForShutdown(const RefPtr<MediaDataEncoder>& aEncoder) {
245251
[&result]() { return result; });
246252
}
247253

248-
static MediaDataEncoder::EncodedData Drain(
254+
static Result<MediaDataEncoder::EncodedData, MediaResult> Drain(
249255
const RefPtr<MediaDataEncoder>& aEncoder) {
256+
MOZ_RELEASE_ASSERT(aEncoder);
257+
250258
size_t pending = 0;
251259
MediaDataEncoder::EncodedData output;
252-
bool succeeded;
253260
do {
254-
media::Await(
255-
GetMediaThreadPool(MediaThreadType::SUPERVISOR), aEncoder->Drain(),
256-
[&pending, &output, &succeeded](MediaDataEncoder::EncodedData encoded) {
257-
pending = encoded.Length();
258-
output.AppendElements(std::move(encoded));
259-
succeeded = true;
260-
},
261-
[&succeeded](const MediaResult& r) { succeeded = false; });
262-
EXPECT_TRUE(succeeded);
263-
if (!succeeded) {
264-
return output;
265-
}
261+
MediaDataEncoder::EncodedData data = MOZ_TRY(WaitFor(aEncoder->Drain()));
262+
pending = data.Length();
263+
output.AppendElements(std::move(data));
266264
} while (pending > 0);
267265

268266
return output;
269267
}
270268

271-
static MediaDataEncoder::EncodedData Encode(
269+
static Result<MediaDataEncoder::EncodedData, MediaResult> Encode(
272270
const RefPtr<MediaDataEncoder>& aEncoder, const size_t aNumFrames,
273271
MediaDataEncoderTest::FrameSource& aSource) {
272+
MOZ_RELEASE_ASSERT(aEncoder);
273+
274274
MediaDataEncoder::EncodedData output;
275-
bool succeeded;
276275
for (size_t i = 0; i < aNumFrames; i++) {
277276
RefPtr<MediaData> frame = aSource.GetFrame(i);
278-
media::Await(
279-
GetMediaThreadPool(MediaThreadType::SUPERVISOR),
280-
aEncoder->Encode(frame),
281-
[&output, &succeeded](MediaDataEncoder::EncodedData encoded) {
282-
output.AppendElements(std::move(encoded));
283-
succeeded = true;
284-
},
285-
[&succeeded](const MediaResult& r) { succeeded = false; });
286-
EXPECT_TRUE(succeeded);
287-
if (!succeeded) {
288-
return output;
289-
}
277+
output.AppendElements(MOZ_TRY(
278+
WaitFor(aEncoder->Encode(frame))));
290279
}
291-
292-
output.AppendElements(Drain(aEncoder));
280+
output.AppendElements(std::move(MOZ_TRY(Drain(aEncoder))));
293281
return output;
294282
}
295283

@@ -380,7 +368,8 @@ static void H264EncodesTest(Usage aUsage,
380368
aUsage, EncoderConfig::SampleFormat(dom::ImageBitmapFormat::YUV420P),
381369
aFrameSource.GetSize(), ScalabilityMode::None, aSpecific);
382370
EnsureInit(e);
383-
MediaDataEncoder::EncodedData output = Encode(e, 1UL, aFrameSource);
371+
MediaDataEncoder::EncodedData output =
372+
GET_OR_RETURN_ON_ERROR(Encode(e, 1UL, aFrameSource));
384373
EXPECT_EQ(output.Length(), 1UL);
385374
EXPECT_TRUE(isAVCC ? AnnexB::IsAVCC(output[0])
386375
: AnnexB::IsAnnexB(output[0]));
@@ -391,7 +380,7 @@ static void H264EncodesTest(Usage aUsage,
391380
aUsage, EncoderConfig::SampleFormat(dom::ImageBitmapFormat::YUV420P),
392381
aFrameSource.GetSize(), ScalabilityMode::None, aSpecific);
393382
EnsureInit(e);
394-
output = Encode(e, NUM_FRAMES, aFrameSource);
383+
output = GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, aFrameSource));
395384
if (aUsage == Usage::Realtime && kImageSize4K <= aFrameSource.GetSize()) {
396385
// Realtime encoding may drop frames for large frame sizes.
397386
EXPECT_LE(output.Length(), NUM_FRAMES);
@@ -472,10 +461,11 @@ static void H264EncodeAfterDrainTest(
472461

473462
EnsureInit(e);
474463

475-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, aFrameSource);
464+
MediaDataEncoder::EncodedData output =
465+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, aFrameSource));
476466
EXPECT_EQ(output.Length(), NUM_FRAMES);
477467

478-
output = Encode(e, NUM_FRAMES, aFrameSource);
468+
output = GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, aFrameSource));
479469
EXPECT_EQ(output.Length(), NUM_FRAMES);
480470

481471
WaitForShutdown(e);
@@ -517,27 +507,15 @@ static void H264InterleavedEncodeAndDrainTest(
517507
EnsureInit(e);
518508

519509
MediaDataEncoder::EncodedData output;
520-
bool succeeded = false;
521510
for (size_t i = 0; i < NUM_FRAMES; i++) {
522511
RefPtr<MediaData> frame = aFrameSource.GetFrame(i);
523-
media::Await(
524-
GetMediaThreadPool(MediaThreadType::SUPERVISOR), e->Encode(frame),
525-
[&output, &succeeded](MediaDataEncoder::EncodedData encoded) {
526-
output.AppendElements(std::move(encoded));
527-
succeeded = true;
528-
},
529-
[&succeeded](const MediaResult& r) { succeeded = false; });
530-
EXPECT_TRUE(succeeded);
531-
if (!succeeded) {
532-
break;
533-
}
534-
512+
output.AppendElements(GET_OR_RETURN_ON_ERROR(WaitFor(e->Encode(frame))));
535513
if (i % 5 == 0) {
536-
output.AppendElements(Drain(e));
514+
output.AppendElements(GET_OR_RETURN_ON_ERROR(Drain(e)));
537515
}
538516
}
517+
output.AppendElements(GET_OR_RETURN_ON_ERROR(Drain(e)));
539518

540-
output.AppendElements(Drain(e));
541519
EXPECT_EQ(output.Length(), NUM_FRAMES);
542520

543521
WaitForShutdown(e);
@@ -569,7 +547,8 @@ TEST_F(MediaDataEncoderTest, H264Duration) {
569547
RUN_IF_SUPPORTED(CodecType::H264, [this]() {
570548
RefPtr<MediaDataEncoder> e = CreateH264Encoder();
571549
EnsureInit(e);
572-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
550+
MediaDataEncoder::EncodedData output =
551+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
573552
EXPECT_EQ(output.Length(), NUM_FRAMES);
574553
for (const auto& frame : output) {
575554
EXPECT_GT(frame->mDuration, media::TimeUnit::Zero());
@@ -609,7 +588,8 @@ TEST_F(MediaDataEncoderTest, H264AVCC) {
609588
EncoderConfig::SampleFormat(dom::ImageBitmapFormat::YUV420P),
610589
kImageSize, ScalabilityMode::None, AsVariant(kH264SpecificAVCC));
611590
EnsureInit(e);
612-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
591+
MediaDataEncoder::EncodedData output =
592+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
613593
EXPECT_EQ(output.Length(), NUM_FRAMES);
614594
for (auto frame : output) {
615595
EXPECT_FALSE(AnnexB::IsAnnexB(frame));
@@ -701,7 +681,8 @@ TEST_F(MediaDataEncoderTest, VP8Encodes) {
701681
// Encode one VPX frame.
702682
RefPtr<MediaDataEncoder> e = CreateVP8Encoder();
703683
EnsureInit(e);
704-
MediaDataEncoder::EncodedData output = Encode(e, 1UL, mData);
684+
MediaDataEncoder::EncodedData output =
685+
GET_OR_RETURN_ON_ERROR(Encode(e, 1UL, mData));
705686
EXPECT_EQ(output.Length(), 1UL);
706687
VPXDecoder::VPXStreamInfo info;
707688
EXPECT_TRUE(
@@ -715,7 +696,7 @@ TEST_F(MediaDataEncoderTest, VP8Encodes) {
715696
// Encode multiple VPX frames.
716697
e = CreateVP8Encoder();
717698
EnsureInit(e);
718-
output = Encode(e, NUM_FRAMES, mData);
699+
output = GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
719700
EXPECT_EQ(output.Length(), NUM_FRAMES);
720701
for (auto frame : output) {
721702
VPXDecoder::VPXStreamInfo info;
@@ -734,7 +715,8 @@ TEST_F(MediaDataEncoderTest, VP8Duration) {
734715
RUN_IF_SUPPORTED(CodecType::VP8, [this]() {
735716
RefPtr<MediaDataEncoder> e = CreateVP8Encoder();
736717
EnsureInit(e);
737-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
718+
MediaDataEncoder::EncodedData output =
719+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
738720
EXPECT_EQ(output.Length(), NUM_FRAMES);
739721
for (const auto& frame : output) {
740722
EXPECT_GT(frame->mDuration, media::TimeUnit::Zero());
@@ -749,7 +731,8 @@ TEST_F(MediaDataEncoderTest, VP8EncodeAfterDrain) {
749731
RefPtr<MediaDataEncoder> e = CreateVP8Encoder();
750732
EnsureInit(e);
751733

752-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
734+
MediaDataEncoder::EncodedData output =
735+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
753736
EXPECT_EQ(output.Length(), NUM_FRAMES);
754737
for (auto frame : output) {
755738
VPXDecoder::VPXStreamInfo info;
@@ -762,7 +745,7 @@ TEST_F(MediaDataEncoderTest, VP8EncodeAfterDrain) {
762745
}
763746
output.Clear();
764747

765-
output = Encode(e, NUM_FRAMES, mData);
748+
output = GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
766749
EXPECT_EQ(output.Length(), NUM_FRAMES);
767750
for (auto frame : output) {
768751
VPXDecoder::VPXStreamInfo info;
@@ -794,7 +777,8 @@ TEST_F(MediaDataEncoderTest, VP8EncodeWithScalabilityModeL1T2) {
794777
EnsureInit(e);
795778

796779
const nsTArray<uint8_t> pattern({0, 1});
797-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
780+
MediaDataEncoder::EncodedData output =
781+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
798782
int temporal_idx = 0;
799783
EXPECT_EQ(output.Length(), NUM_FRAMES);
800784
for (size_t i = 0; i < output.Length(); ++i) {
@@ -826,7 +810,8 @@ TEST_F(MediaDataEncoderTest, VP8EncodeWithScalabilityModeL1T3) {
826810
EnsureInit(e);
827811

828812
const nsTArray<uint8_t> pattern({0, 2, 1, 2});
829-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
813+
MediaDataEncoder::EncodedData output =
814+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
830815
EXPECT_EQ(output.Length(), NUM_FRAMES);
831816
int temporal_idx = 0;
832817
for (size_t i = 0; i < output.Length(); ++i) {
@@ -872,7 +857,8 @@ TEST_F(MediaDataEncoderTest, VP9Encodes) {
872857
RUN_IF_SUPPORTED(CodecType::VP9, [this]() {
873858
RefPtr<MediaDataEncoder> e = CreateVP9Encoder();
874859
EnsureInit(e);
875-
MediaDataEncoder::EncodedData output = Encode(e, 1UL, mData);
860+
MediaDataEncoder::EncodedData output =
861+
GET_OR_RETURN_ON_ERROR(Encode(e, 1UL, mData));
876862
EXPECT_EQ(output.Length(), 1UL);
877863
VPXDecoder::VPXStreamInfo info;
878864
EXPECT_TRUE(
@@ -885,7 +871,7 @@ TEST_F(MediaDataEncoderTest, VP9Encodes) {
885871

886872
e = CreateVP9Encoder();
887873
EnsureInit(e);
888-
output = Encode(e, NUM_FRAMES, mData);
874+
output = GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
889875
EXPECT_EQ(output.Length(), NUM_FRAMES);
890876
for (auto frame : output) {
891877
VPXDecoder::VPXStreamInfo info;
@@ -904,7 +890,8 @@ TEST_F(MediaDataEncoderTest, VP9Duration) {
904890
RUN_IF_SUPPORTED(CodecType::VP9, [this]() {
905891
RefPtr<MediaDataEncoder> e = CreateVP9Encoder();
906892
EnsureInit(e);
907-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
893+
MediaDataEncoder::EncodedData output =
894+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
908895
EXPECT_EQ(output.Length(), NUM_FRAMES);
909896
for (const auto& frame : output) {
910897
EXPECT_GT(frame->mDuration, media::TimeUnit::Zero());
@@ -919,7 +906,8 @@ TEST_F(MediaDataEncoderTest, VP9EncodeAfterDrain) {
919906
RefPtr<MediaDataEncoder> e = CreateVP9Encoder();
920907
EnsureInit(e);
921908

922-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
909+
MediaDataEncoder::EncodedData output =
910+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
923911
EXPECT_EQ(output.Length(), NUM_FRAMES);
924912
for (auto frame : output) {
925913
VPXDecoder::VPXStreamInfo info;
@@ -932,7 +920,7 @@ TEST_F(MediaDataEncoderTest, VP9EncodeAfterDrain) {
932920
}
933921
output.Clear();
934922

935-
output = Encode(e, NUM_FRAMES, mData);
923+
output = GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
936924
EXPECT_EQ(output.Length(), NUM_FRAMES);
937925
for (auto frame : output) {
938926
VPXDecoder::VPXStreamInfo info;
@@ -968,7 +956,8 @@ TEST_F(MediaDataEncoderTest, VP9EncodeWithScalabilityModeL1T2) {
968956
EnsureInit(e);
969957

970958
const nsTArray<uint8_t> pattern({0, 1});
971-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
959+
MediaDataEncoder::EncodedData output =
960+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
972961
int temporal_idx = 0;
973962
EXPECT_EQ(output.Length(), NUM_FRAMES);
974963
for (size_t i = 0; i < output.Length(); ++i) {
@@ -1004,7 +993,8 @@ TEST_F(MediaDataEncoderTest, VP9EncodeWithScalabilityModeL1T3) {
1004993
EnsureInit(e);
1005994

1006995
const nsTArray<uint8_t> pattern({0, 2, 1, 2});
1007-
MediaDataEncoder::EncodedData output = Encode(e, NUM_FRAMES, mData);
996+
MediaDataEncoder::EncodedData output =
997+
GET_OR_RETURN_ON_ERROR(Encode(e, NUM_FRAMES, mData));
1008998
int temporal_idx = 0;
1009999
EXPECT_EQ(output.Length(), NUM_FRAMES);
10101000
for (size_t i = 0; i < output.Length(); ++i) {
@@ -1021,3 +1011,6 @@ TEST_F(MediaDataEncoderTest, VP9EncodeWithScalabilityModeL1T3) {
10211011
}
10221012
# endif
10231013
#endif
1014+
1015+
#undef GET_OR_RETURN_ON_ERROR
1016+
#undef RUN_IF_SUPPORTED

‎dom/media/systemservices/MediaUtils.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "mozilla/MozPromise.h"
1616
#include "mozilla/Mutex.h"
1717
#include "mozilla/RefPtr.h"
18+
#include "mozilla/Result.h"
1819
#include "mozilla/SharedThreadPool.h"
1920
#include "mozilla/TaskQueue.h"
2021
#include "mozilla/UniquePtr.h"

‎testing/gtest/mozilla/WaitFor.h‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,10 +89,13 @@ template <typename R, typename E, bool Exc>
8989
inline Result<R, E> WaitFor(const RefPtr<MozPromise<R, E, Exc>>& aPromise) {
9090
Maybe<R> success;
9191
Maybe<E> error;
92+
// Use r-value reference for exclusive promises to support move-only types.
93+
using RRef = typename std::conditional_t<Exc, R&&, const R&>;
94+
using ERef = typename std::conditional_t<Exc, E&&, const E&>;
9295
aPromise->Then(
9396
GetCurrentSerialEventTarget(), __func__,
94-
[&](R aResult) { success = Some(aResult); },
95-
[&](E aError) { error = Some(aError); });
97+
[&](RRef aResult) { success.emplace(std::forward<RRef>(aResult)); },
98+
[&](ERef aError) { error.emplace(std::forward<ERef>(aError)); });
9699
SpinEventLoopUntil<ProcessFailureBehavior::IgnoreAndContinue>(
97100
"WaitFor(const RefPtr<MozPromise<R, E, Exc>>& aPromise)"_ns,
98101
[&] { return success.isSome() || error.isSome(); });

0 commit comments

Comments
 (0)