#18 Valgrind Error in TheoraVideoClip_Theora::load of TheoraVideoClip_Theora.cpp

[Anonymous]

The TheoraVideoClip_Theora::load method can generate Valgrind errors. The offending code starts at line 247 of TheoraVideoClip_Theora.cpp:

---

for (;;)
{
int ret = ogg_sync_pageout(&mInfo.OggSyncState, &mInfo.OggPage);
if (ret == 0) break;
// if page is not a theora page, skip it
if (ogg_page_serialno(&mInfo.OggPage) != mInfo.TheoraStreamState.serialno) continue;

---

The first error is generated at line 252 (call to ogg_page_serialno) and can be reproduced by playing the Big Buck Bunny video.

---

==12788== Invalid read of size 1
==12788== at 0x22A7A8: ogg_page_serialno
==12788== by 0x23152A: TheoraVideoClip_Theora::load(TheoraDataSource)
==12788== by 0x23486A: TheoraVideoManager::createVideoClip(TheoraDataSource, TheoraOutputMode, int, bool)
==12788== by 0x234A6A: TheoraVideoManager::createVideoClip(std::string, TheoraOutputMode, int, bool)

---

After this error, Valgrind reports another one.

---

==12788== Address 0x703c60a is 7,210 bytes inside a block of size 8,192 free'd
==12788== at 0x394398: realloc (vg_replace_malloc.c:666)
==12788== by 0x22AF81: ogg_sync_buffer
==12788== by 0x2314D5: TheoraVideoClip_Theora::load(TheoraDataSource)
==12788== by 0x23486A: TheoraVideoManager::createVideoClip(TheoraDataSource, TheoraOutputMode, int, bool)
==12788== by 0x234A6A: TheoraVideoManager::createVideoClip(std::string, TheoraOutputMode, int, bool)

---

I believe this is because the above code fails to handle the situation when ogg_sync_pageout returns a negative value. Changing the code as shown below avoids the error.

---

for (;;)
{
int ret = ogg_sync_pageout(&mInfo.OggSyncState, &mInfo.OggPage);
if (ret == 0) break;
else if (ret < 0) continue; / avoids the Valgrind errors /

// if page is not a theora page, skip it
if (ogg_page_serialno(&mInfo.OggPage) != mInfo.TheoraStreamState.serialno) continue;

---

With this change Valgrind no longer reports any errors and the video still loads and plays fine. However, I'm not sure if this fix is correct.