Skip to content
Snippets Groups Projects

FixForTicket#891

Merged Hendry requested to merge hendry197/VVCSoftware_VTM:FixForTicket891 into master
1 unresolved thread

Change the checking for missing pictures to check only active reference picture, not all pictures in the RPS

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
1871 1871 DTRACE_UPDATE( g_trace_ctx, std::make_pair( "final", 1 ) );
1872 1872 }
1873 1873
1874 #if FIX_TICKET891
1875 // actual decoding starts here
1876 xActivateParameterSets( nalu.m_nuhLayerId );
  • Contributor

    1.) I certainly may be overlooking something, but is there are reason that xActivateParameterSets needed to be moved earlier for this bug fix? To get the number of active references from the PPS? I believe those already should be set during slice parsing (using the PPS as needed). No?

    The reason I ask is that moving this earlier has side effects. For example:

    • The following line within xActivateParameterSets swaps out the SlicePilot, so all the remaining references to SlicePilot made after this point are now invalid.

      m_apcSlicePilot = m_pcPic->swapSliceObject(m_apcSlicePilot, m_uiSliceSegmentIdx);

    If there's not a strong reason to move it, maybe it's better to leave it where it was to avoid debugging and fixing any such side effects?

    2.) When applyReferencePictureListBasedMarking( ) is called within xActivateParameterSets( ), it assumes the long-term reference pictures have already been marked as long-term within checkThatAllRefPicsAreAvailable( ). If this bug-fix is going to:

    • Reverse the order of checkThatAllRefPicsAreAvailable and applyReferencePictureListBasedMarking
    • Only call checkThatAllRefPicsAreAvailable on the active references

    then the long-term marking process will be broken and need to be reconsidered. Probably need to move the marking process from checkThatAllRefPicsAreAvailable into applyReferencePictureListBasedMarking.

  • Author Contributor

    Hi Brian, The reason for moving the call to xActivateParameterSets earlier is to ensure I got correct number of active ref pics. Plus, that is the right sequence of logic I think.

    However, looking at the codes, your concerns are valid. Particularly about the swapping of objects from slice pilot to the current slice. To fix these things, I am afraid there will be many changes needed.

    Thus, I think your suggestion to put that call to xActivateParameterSets to its previous position is good solution. Plus, my need is already fulfilled as the number of active ref pics have been set correctly anyway. I confirm after checking the parsing process.

    I'll submit an MR for put back this line of code to its earlier position.

  • It seems xActivateParameterSets() became quite a beast over time. I already moved out the conformance checks into an own function recently. But since it's doing a lot more than just activating parameter sets, it may be good to split up at some point.

  • Author Contributor

    Indeed there are many stuff are done inside that function. Cleaning this up (e.g., split up at some point) can be done. Don't call me out if I don't do it but when I have more time, I can try to take a look inside it more detail ^__^

    btw, MR !1423 (merged) contains the code to put back the call to this function to earlier position

    Edited by Hendry
  • Please register or sign in to reply
Please register or sign in to reply
Loading