Possible unintended var usage in file "chromium/media/muxers/webm_muxer.cc" line 210
Reported by
pet...@gmail.com,
Jul 3
|
|||
Issue description
UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/67.0.3396.99 Safari/537.36
Steps to reproduce the problem:
While experimenting with a CodeSonar plugin we develop, we noticed a potential bug in file "chromium/media/muxers/webm_muxer.cc" line 210 function WebmMuxer::OnEncodedAudio
while (!encoded_frames_queue_.empty()) {
const bool res = AddFrame(
std::make_unique<std::string>(*encoded_frames_queue_.front()->data),
encoded_frames_queue_.front()->alpha_data
? std::make_unique<std::string>(
*encoded_frames_queue_.front()->alpha_data)
: nullptr,
video_track_index_, //HERE
encoded_frames_queue_.front()->timestamp - first_frame_timestamp_video_,
encoded_frames_queue_.front()->is_keyframe);
if (!res)
return false;
encoded_frames_queue_.pop_front();
}
Shouldn't audio_track_index_ be used instead of video_track_index_? Similar for first_frame_timestamp_video_ (next line), shouldn't first_frame_timestamp_audio_ be used?
Thanks,
Petru Florin Mihancea
What is the expected behavior?
This potential problem has been detected automatically via static analysis.
What went wrong?
This potential problem has been detected automatically via static analysis.
Did this work before? N/A
Chrome version: 67.0.3396.99 Channel: stable
OS Version: OS X 10.13.5
Flash Version:
,
Jul 6
The issue seems to be out of TE-scope as it is related to chromium/media/muxers/webm_muxer.cc file. Hence, adding label TE-NeedsTriageHelp for further investigation from dev team. Thanks...!!
,
Jul 9
petrum@ thanks for reporting. Assuming you refer to [1]. One limitation of the Webm Muxer is that all tracks info has to be written before actual data is present for either; moreover, if encoded audio is expected to be present, we have to buffer up the encoded video so as not to lose the last key frame (see e.g. what happens in [2]). To reconcile the fact that encoded audio and video can and usually do arrive at WebmMuxer at different times, encoded video is kept in |encoded_frames_queue_| in [3] (again, if audio is expected). When encoded audio is received, the encoded video is AddFrame()d in [4] before the encoded audio itself is AddFrame()d in [5]. So, the line you mention is correct in using |video_track_index_|. Does this make sense? Tentatively marking as WontFix, but please feel free to expose your thoughts and/or correct me. [1] https://cs.chromium.org/chromium/src/media/muxers/webm_muxer.cc?type=cs&q=media/muxers/webm_muxer.cc&sq=package:chromium&g=0&l=203 [2] https://cs.chromium.org/chromium/src/media/muxers/webm_muxer.cc?type=cs&q=media/muxers/webm_muxer.cc&sq=package:chromium&g=0&l=167 [3] https://cs.chromium.org/chromium/src/media/muxers/webm_muxer.cc?type=cs&q=media/muxers/webm_muxer.cc&sq=package:chromium&g=0&l=164 [4] https://cs.chromium.org/chromium/src/media/muxers/webm_muxer.cc?type=cs&q=media/muxers/webm_muxer.cc&sq=package:chromium&g=0&l=204 [5] https://cs.chromium.org/chromium/src/media/muxers/webm_muxer.cc?type=cs&q=media/muxers/webm_muxer.cc&sq=package:chromium&g=0&l=217
,
Jul 11
Hi, Yes, I think I got the idea and it really makes sense :). Thanks, Petru-Florin Mihancea |
|||
►
Sign in to add a comment |
|||
Comment 1 by dtapu...@chromium.org
, Jul 4