Remove multi-stream related code #483
Conversation
facebook-github-bot
added
the
CLA Signed
|
|
||
| return AVFrameStream(std::move(avFrame), frameStreamIndex); | ||
| return AVFrameStream(std::move(avFrame), activeStreamIndex_); |
There was a problem hiding this comment.
We don't really need this AVFrameStream struct anymore, since we know the streamIndex is always activeStreamIndex_. We could remove it, but I guess this is better done in another PR.
| @@ -907,15 +864,14 @@ VideoDecoder::AVFrameStream VideoDecoder::decodeAVFrame( | |||
| getFFMPEGErrorStringFromErrorCode(ffmpegStatus)); | |||
| } | |||
|
|
|||
| if (activeStreamIndices_.count(packet->stream_index) == 0) { | |||
| // This packet is not for any of the active streams. | |||
| if (packet->stream_index != activeStreamIndex_) { | |||
There was a problem hiding this comment.
BTW, this is the "demux" part now. And I think we should be calling av_read_frame within its own while loop, for as long as the received packet isn't of the target stream.
There was a problem hiding this comment.
Yeah, what we're currently doing is inefficient, because if the packet is not the right stream, we first have to make a call to avcodec_receive_frame() that we know will fail. We're still doing the correct thing (I think), it's just inefficient.
| // Stores the stream indices of the active streams, i.e. the streams we are | ||
| // decoding and returning to the user. | ||
| std::set<int> activeStreamIndices_; | ||
| int activeStreamIndex_ = -1; |
There was a problem hiding this comment.
I'd prefer to both make this a named constant, and to use something other than -1. Perhaps:
const int NO_ACTIVE_STREAM = -2;
Obviously 0 and above are potential active stream numbers, so they can't be used as such an indicator. But I'm worried that -1 can get confusing, because that is used to ask FFmpeg to find the best stream in some API calls. So I'd like to also treat -1 as a special stream value.
Towards #476
The main point of this PR is to raise an error when the usert tries to add more than one stream. This is a bug fix: previously, we weren't erroring, but we were returning silently wrong results as described in #476.
In addition, we remove multi-stream logic in this PR: we don't support multiple streams or multi-plexing, so there's no point pretending. Effectively, this removes the
activeStreamIndices_set and replaces it withint activeStreamIndex_. We keep thestreamInfos_map and it is unchanged, as its logic is relevant for metadata.