Skip to content

Commit

Permalink
VP8: Fix and improve temporal layer selection
Browse files Browse the repository at this point in the history
Fixes versatica#989

* An old payload cannot upgrade the current temporal layer.
* Update current temporal layer to the target one.
  • Loading branch information
jmillan committed Jan 31, 2023
1 parent c942fd9 commit 13d850b
Show file tree
Hide file tree
Showing 4 changed files with 125 additions and 19 deletions.
5 changes: 5 additions & 0 deletions worker/include/RTC/SeqManager.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,14 @@ namespace RTC
void Drop(T input);
void Offset(T offset);
bool Input(const T input, T& output);
bool IsSeqLowerThanMaxInput(const T input) const;
bool IsSeqHigherThanMaxInput(const T input) const;
T GetMaxInput() const;
T GetMaxOutput() const;

private:
bool Started() const;

private:
T base{ 0 };
T maxOutput{ 0 };
Expand Down
43 changes: 24 additions & 19 deletions worker/src/RTC/Codecs/VP8.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,13 @@ namespace RTC
MS_WARN_DEV("stream is supposed to have >1 temporal layers but does not have TlIndex field");
}

auto oldPictureId =
context->pictureIdManager.IsSeqLowerThanMaxInput(this->payloadDescriptor->pictureId);

// Check whether pictureId and tl0PictureIndex sync is required.
// clang-format off
if (
!oldPictureId &&
context->syncRequired &&
this->payloadDescriptor->hasPictureId &&
this->payloadDescriptor->hasTl0PictureIndex
Expand All @@ -255,12 +259,10 @@ namespace RTC
// Incremental pictureId. Check the temporal layer.
// clang-format off
if (
!oldPictureId &&
this->payloadDescriptor->hasPictureId &&
this->payloadDescriptor->hasTlIndex &&
this->payloadDescriptor->hasTl0PictureIndex &&
!RTC::SeqManager<uint16_t, 15>::IsSeqLowerThan(
this->payloadDescriptor->pictureId,
context->pictureIdManager.GetMaxInput())
this->payloadDescriptor->hasTl0PictureIndex
)
// clang-format on
{
Expand Down Expand Up @@ -321,23 +323,26 @@ namespace RTC
return false;
}

// Update/fix current temporal layer.
// clang-format off
if (
this->payloadDescriptor->hasTlIndex &&
this->payloadDescriptor->tlIndex > context->GetCurrentTemporalLayer()
)
// clang-format on
{
context->SetCurrentTemporalLayer(this->payloadDescriptor->tlIndex);
}
else if (!this->payloadDescriptor->hasTlIndex)
if (!oldPictureId)
{
context->SetCurrentTemporalLayer(0);
}
// Update/fix current temporal layer.
// clang-format off
if (
this->payloadDescriptor->hasTlIndex &&
this->payloadDescriptor->tlIndex == context->GetTargetTemporalLayer()
)
// clang-format on
{
context->SetCurrentTemporalLayer(this->payloadDescriptor->tlIndex);
}
else if (!this->payloadDescriptor->hasTlIndex)
{
context->SetCurrentTemporalLayer(0);
}

if (context->GetCurrentTemporalLayer() > context->GetTargetTemporalLayer())
context->SetCurrentTemporalLayer(context->GetTargetTemporalLayer());
if (context->GetCurrentTemporalLayer() > context->GetTargetTemporalLayer())
context->SetCurrentTemporalLayer(context->GetTargetTemporalLayer());
}

// clang-format off
if (
Expand Down
30 changes: 30 additions & 0 deletions worker/src/RTC/SeqManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,36 @@ namespace RTC
return this->maxOutput;
}

template<typename T, uint8_t N>
bool SeqManager<T, N>::IsSeqLowerThanMaxInput(const T input) const
{
// Not started. Input cannot be lower than nothing.
if (!this->Started())
{
return false;
}

return RTC::SeqManager<T, N>::IsSeqLowerThan(input, this->maxInput);
}

template<typename T, uint8_t N>
bool SeqManager<T, N>::IsSeqHigherThanMaxInput(const T input) const
{
// Not started. Input is higher than nothing.
if (!this->Started())
{
return true;
}

return RTC::SeqManager<T, N>::IsSeqHigherThan(input, this->maxInput);
}

template<typename T, uint8_t N>
bool SeqManager<T, N>::Started() const
{
return (this->base != 0 || this->maxOutput != 0 || this->maxInput != 0);
}

// Explicit instantiation to have all SeqManager definitions in this file.
template class SeqManager<uint8_t>;
template class SeqManager<uint16_t>;
Expand Down
66 changes: 66 additions & 0 deletions worker/test/src/RTC/Codecs/TestVP8.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -322,4 +322,70 @@ SCENARIO("process VP8 payload descriptor", "[codecs][vp8]")
forwarded = ProcessPacket(context, 1, 0, 1);
REQUIRE_FALSE(forwarded);
}

SECTION("old packet with higher temporal layer than current does not change current temporal layer")
{
RTC::Codecs::EncodingContext::Params params;
params.spatialLayers = 0;
params.temporalLayers = 2;
Codecs::VP8::EncodingContext context(params);
context.SyncRequired();

context.SetCurrentTemporalLayer(0);
context.SetTargetTemporalLayer(0);

// Frame 1.
auto forwarded = ProcessPacket(context, 1, 0, 0);
REQUIRE(forwarded);
REQUIRE(forwarded->pictureId == 1);
REQUIRE(forwarded->tlIndex == 0);
REQUIRE(forwarded->tl0PictureIndex == 1);

// Frame 2.
forwarded = ProcessPacket(context, 2, 0, 0);
REQUIRE(forwarded);
REQUIRE(forwarded->pictureId == 2);
REQUIRE(forwarded->tlIndex == 0);
REQUIRE(forwarded->tl0PictureIndex == 1);

// Frame 3. Old packet with higher temporal layer.
forwarded = ProcessPacket(context, 0, 0, 1);
REQUIRE(forwarded);
REQUIRE(forwarded->pictureId == 0);
REQUIRE(forwarded->tlIndex == 1);
REQUIRE(context.GetCurrentTemporalLayer() == 0);
}

SECTION("packet with higher temporal layer than target does not change current temporal layer")
{
RTC::Codecs::EncodingContext::Params params;
params.spatialLayers = 0;
params.temporalLayers = 2;
Codecs::VP8::EncodingContext context(params);
context.SyncRequired();

context.SetCurrentTemporalLayer(0);
context.SetTargetTemporalLayer(0);

// Frame 1.
auto forwarded = ProcessPacket(context, 1, 0, 0);
REQUIRE(forwarded);
REQUIRE(forwarded->pictureId == 1);
REQUIRE(forwarded->tlIndex == 0);
REQUIRE(forwarded->tl0PictureIndex == 1);

// Frame 2.
forwarded = ProcessPacket(context, 2, 0, 0);
REQUIRE(forwarded);
REQUIRE(forwarded->pictureId == 2);
REQUIRE(forwarded->tlIndex == 0);
REQUIRE(forwarded->tl0PictureIndex == 1);

context.SetTargetTemporalLayer(2);

// Frame 3. Packet with higher temporal layer than target.
forwarded = ProcessPacket(context, 3, 0, 3, true /*layerSync*/);
REQUIRE_FALSE(forwarded);
REQUIRE(context.GetCurrentTemporalLayer() == 0);
}
}

0 comments on commit 13d850b

Please sign in to comment.