CDVDDemuxFFmpeg: also check for extradata_size change before adding a disabled stream

[samnazarko]

Description

This issue was discovered when implementing 3D MVC support.

How has this been tested?

This has been tested on OSMC Vero 4K / 4K + devices since 2020.

What is the effect on users?

Screenshots (if appropriate):

Types of change

• Bug fix (non-breaking change which fixes an issue)
• Clean up (non-breaking change which removes non-working, unmaintained functionality)
• Improvement (non-breaking change which improves existing functionality)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that will cause existing functionality to change)
• Cosmetic change (non-breaking change that doesn't touch code)
• None of the above (please explain below)

Checklist:

• My code follows the Code Guidelines of this project
• My change requires a change to the documentation, either Doxygen or wiki
• I have updated the documentation accordingly
• I have read the Contributing document
• I have added tests to cover my change
• All new and existing tests passed

[lrusak]

Seems ok. I'm curious though when this would happen?

[lrusak]

20:15:48 /home/jenkins/workspace/FreeBSD/xbmc/cores/VideoPlayer/DVDDemuxers/DVDDemuxFFmpeg.cpp:1227:44: error: comparison of integers of different signs: 'unsigned int' and 'int' [-Werror,-Wsign-compare]
20:15:48                   (stream->disabled && stream->ExtraSize != m_pFormatContext->streams[pPacket->iStreamId]->codecpar->extradata_size))
20:15:48                                        ~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
20:15:48 1 error generated.

[jenkins4kodi]

I've made some formatting changes to meet the current code style. The diffs are available in the following links:

• PR23173-0001-38b91b5a1b-CDVDDemuxFFmpeg-also-check-for-ext.diff
• PR23173.diff

For more information please see our current code style guidelines.

[AlwinEsch]

@lrusak what you think to make as (to prevent unneeded compares):

      if (!stream->disabled && (static_cast<CDemuxStreamVideo*>(stream)->iWidth != m_pFormatContext->streams[pPacket->iStreamId]->codecpar->width ||
          static_cast<CDemuxStreamVideo*>(stream)->iHeight != m_pFormatContext->streams[pPacket->iStreamId]->codecpar->height) ||
		  stream->ExtraSize != m_pFormatContext->streams[pPacket->iStreamId]->codecpar->extradata_size)

Or to have more short and less in mem works:

      const auto codecpar = m_pFormatContext->streams[pPacket->iStreamId]->codecpar;
      const auto videostream = static_cast<CDemuxStreamVideo*>(stream);
      if (!stream->disabled && (videostream->iWidth != codecpar->width ||
          videostream->iHeight != codecpar->height) ||
	  videostream->ExtraSize != codecpar->extradata_size)

Due to missing compiler not tested now!

Thanks @samnazarko, good improvement, can be become bad if a extradata becomes not given complete in all cases

[AlwinEsch]

Looked in the whole function, about the codecpar, maybe the best on begin of "if":

  // check streams, can we make this a bit more simple?
  if (pPacket->iStreamId >= 0)
  {
    const auto codecpar = m_pFormatContext->streams[pPacket->iStreamId]->codecpar;
    CDemuxStream* stream = GetStream(pPacket->iStreamId);
    ...

and then maybe better as separate request?