-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix issues in BitstreamConverter #23174
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -535,7 +535,7 @@ bool CBitstreamConverter::Convert(uint8_t *pData, int iSize) | |||||||||||
if (m_convert_bitstream) | ||||||||||||
{ | ||||||||||||
// convert demuxer packet from bitstream to bytestream (AnnexB) | ||||||||||||
int bytestream_size = 0; | ||||||||||||
uint32_t bytestream_size = 0; | ||||||||||||
uint8_t *bytestream_buff = NULL; | ||||||||||||
|
||||||||||||
BitstreamConvert(demuxer_content, demuxer_bytes, &bytestream_buff, &bytestream_size); | ||||||||||||
|
@@ -678,6 +678,7 @@ bool CBitstreamConverter::BitstreamConvertInitAVC(void *in_extradata, int in_ext | |||||||||||
uint16_t unit_size; | ||||||||||||
uint32_t total_size = 0; | ||||||||||||
uint8_t *out = NULL, unit_nb, sps_done = 0, sps_seen = 0, pps_seen = 0; | ||||||||||||
uint8_t mvc_done = 0; | ||||||||||||
const uint8_t *extradata = (uint8_t*)in_extradata + 4; | ||||||||||||
static const uint8_t nalu_header[4] = {0, 0, 0, 1}; | ||||||||||||
|
||||||||||||
|
@@ -726,6 +727,18 @@ bool CBitstreamConverter::BitstreamConvertInitAVC(void *in_extradata, int in_ext | |||||||||||
if (unit_nb) | ||||||||||||
pps_seen = 1; | ||||||||||||
} | ||||||||||||
|
||||||||||||
if (!unit_nb && !mvc_done++) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know you were following the pattern but generally ints either need to be compared directly or casted to bool. I also recommend using a bool when wanting a boolean check.
Suggested change
|
||||||||||||
{ | ||||||||||||
if (in_extrasize - total_size > 14 && memcmp(extradata + 8, "mvcC", 4) == 0) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally magic numbers are frowned upon. Please use constexpr int's to help with human readable values. In reality it would just be nice if we could define a structure for the extradata so we don't need to do all this magic number jumping around but I'll leave that as a future wish.
Suggested change
|
||||||||||||
{ | ||||||||||||
// start over; take SPS and PPS from the mvcC atom | ||||||||||||
extradata += 12 + 5; // skip over mvcC atom header | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. magic everywhere! |
||||||||||||
unit_nb = *extradata++ & 0x1f; // number of sps unit(s) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. more magic? |
||||||||||||
sps_done = 0; | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be a gold star if you could (possibly in a future PR) convert these to bool. |
||||||||||||
pps_seen = 0; | ||||||||||||
} | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
if (out) | ||||||||||||
|
@@ -868,7 +881,7 @@ bool CBitstreamConverter::IsSlice(uint8_t unit_type) | |||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
bool CBitstreamConverter::BitstreamConvert(uint8_t* pData, int iSize, uint8_t **poutbuf, int *poutbuf_size) | ||||||||||||
bool CBitstreamConverter::BitstreamConvert(uint8_t* pData, int iSize, uint8_t **poutbuf, uint32_t *poutbuf_size) | ||||||||||||
{ | ||||||||||||
// based on h264_mp4toannexb_bsf.c (ffmpeg) | ||||||||||||
// which is Copyright (c) 2007 Benoit Fouet <benoit.fouet@free.fr> | ||||||||||||
|
@@ -878,7 +891,7 @@ bool CBitstreamConverter::BitstreamConvert(uint8_t* pData, int iSize, uint8_t ** | |||||||||||
uint8_t *buf = pData; | ||||||||||||
uint32_t buf_size = iSize; | ||||||||||||
uint8_t unit_type, nal_sps, nal_pps, nal_sei; | ||||||||||||
int32_t nal_size; | ||||||||||||
uint32_t nal_size; | ||||||||||||
uint32_t cumul_size = 0; | ||||||||||||
const uint8_t *buf_end = buf + buf_size; | ||||||||||||
|
||||||||||||
|
@@ -916,7 +929,7 @@ bool CBitstreamConverter::BitstreamConvert(uint8_t* pData, int iSize, uint8_t ** | |||||||||||
unit_type = (*buf >> 1) & 0x3f; | ||||||||||||
} | ||||||||||||
|
||||||||||||
if (buf + nal_size > buf_end || nal_size <= 0) | ||||||||||||
if (nal_size > (buf_end - buf) || nal_size == 0) | ||||||||||||
goto fail; | ||||||||||||
|
||||||||||||
// Don't add sps/pps if the unit already contain them | ||||||||||||
|
@@ -956,7 +969,7 @@ bool CBitstreamConverter::BitstreamConvert(uint8_t* pData, int iSize, uint8_t ** | |||||||||||
} | ||||||||||||
|
||||||||||||
void CBitstreamConverter::BitstreamAllocAndCopy(uint8_t** poutbuf, | ||||||||||||
int* poutbuf_size, | ||||||||||||
uint32_t* poutbuf_size, | ||||||||||||
const uint8_t* sps_pps, | ||||||||||||
uint32_t sps_pps_size, | ||||||||||||
const uint8_t* in, | ||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.