search

medooze / media-server

WebRTC Media Server
GNU General Public License v2.0
1.36k stars 295 forks source link

VideoDecoder: accept MediaFrames, not RTPPackets #271

Closed mildsunrise closed 6 months ago

mildsunrise commented 8 months ago

this commit makes VideoDecoderWorker implement MediaFrame::Listener interface instead of RTPIncomingMediaStream::Listener. unless I'm missing something, the change looks pretty straightforward to me since the interfaces are very similar and map well.

this change drops the loss detection logic and the partial frame retaining logic. loss detection is only used to update the waitIntra flag, which is just used for logging.

these two features (loss detection and partial frame retaining) belong in the depacketizers, not here. if they are added in the future to the depacketizers, we can add boolean attributes to MediaFrame to signal frame loss or partial frames, and then all that would be needed in VideoDecoderWorker is to check those flags and update waitIntra as needed.

mildsunrise commented 8 months ago

lgtm, but we should clean the VideoDecoder interface and implementations as well

yes, I was planning to do that afterwards (and also replicate the changes in audio, too). this was just a proof of concept. let me test it and I'll do the VideoDecoder refactor

murillo128 commented 7 months ago

let's prioritize this PR as I need to make other changes to the VideoEncoder which will benefit for this refactor

mildsunrise commented 7 months ago

@bcostdolby you said you would take over on this, did you start work yet? if not, I can continue working on the branch once I'm done with the initial version of the filter

@murillo128 which changes are you planning to make on VideoEncoder?

bcostdolby commented 6 months ago

@bcostdolby you said you would take over on this, did you start work yet? if not, I can continue working on the branch once I'm done with the initial version of the filter

@murillo128 which changes are you planning to make on VideoEncoder?

No. I have not started on this issue yet. If you have time now then please take it.

murillo128 commented 6 months ago

I have continued the work here: https://github.com/medooze/media-server/pull/281

mildsunrise commented 6 months ago

reopening to rebase against latest changes