Skip to content

Commit

Permalink
dvdplayer: sub_id in ffmpeg has been depreciated and doesn't work
Browse files Browse the repository at this point in the history
  • Loading branch information
@elupus
elupus committed Apr 11, 2013
1 parent 3d2b0a8 commit 8967acd
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 13 deletions.
1 change: 0 additions & 1 deletion xbmc/cores/dvdplayer/DVDCodecs/Overlay/DVDOverlayCodecFFmpeg.cpp
View file
Expand Up @@ -60,7 +60,6 @@ bool CDVDOverlayCodecFFmpeg::Open(CDVDStreamInfo &hints, CDVDCodecOptions &optio
m_pCodecContext->debug_mv = 0; m_pCodecContext->debug_mv = 0;
m_pCodecContext->debug = 0; m_pCodecContext->debug = 0;
m_pCodecContext->workaround_bugs = FF_BUG_AUTODETECT; m_pCodecContext->workaround_bugs = FF_BUG_AUTODETECT;
m_pCodecContext->sub_id = hints.identifier;
m_pCodecContext->codec_tag = hints.codec_tag; m_pCodecContext->codec_tag = hints.codec_tag;
m_pCodecContext->time_base.num = 1; m_pCodecContext->time_base.num = 1;
m_pCodecContext->time_base.den = DVD_TIME_BASE; m_pCodecContext->time_base.den = DVD_TIME_BASE;
Expand Down
3 changes: 0 additions & 3 deletions xbmc/cores/dvdplayer/DVDDemuxers/DVDDemux.h
View file
Expand Up @@ -207,11 +207,8 @@ class CDemuxStreamSubtitle : public CDemuxStream
public: public:
CDemuxStreamSubtitle() : CDemuxStream() CDemuxStreamSubtitle() : CDemuxStream()
{ {
identifier = 0;
type = STREAM_SUBTITLE; type = STREAM_SUBTITLE;
} }

int identifier;
}; };


class CDemuxStreamTeletext : public CDemuxStream class CDemuxStreamTeletext : public CDemuxStream
Expand Down
1 change: 0 additions & 1 deletion xbmc/cores/dvdplayer/DVDDemuxers/DVDDemuxFFmpeg.cpp
View file
Expand Up @@ -1046,7 +1046,6 @@ void CDVDDemuxFFmpeg::AddStream(int iId)
{ {
CDemuxStreamSubtitleFFmpeg* st = new CDemuxStreamSubtitleFFmpeg(this, pStream); CDemuxStreamSubtitleFFmpeg* st = new CDemuxStreamSubtitleFFmpeg(this, pStream);
m_streams[iId] = st; m_streams[iId] = st;
st->identifier = pStream->codec->sub_id;


if(m_dllAvUtil.av_dict_get(pStream->metadata, "title", NULL, 0)) if(m_dllAvUtil.av_dict_get(pStream->metadata, "title", NULL, 0))
st->m_description = m_dllAvUtil.av_dict_get(pStream->metadata, "title", NULL, 0)->value; st->m_description = m_dllAvUtil.av_dict_get(pStream->metadata, "title", NULL, 0)->value;
Expand Down
8 changes: 7 additions & 1 deletion xbmc/cores/dvdplayer/DVDDemuxers/DVDDemuxHTSP.cpp
View file
Expand Up @@ -316,7 +316,13 @@ void CDVDDemuxHTSP::SubscriptionStart (htsmsg_t *m)
uint32_t composition_id = 0, ancillary_id = 0; uint32_t composition_id = 0, ancillary_id = 0;
htsmsg_get_u32(sub, "composition_id", &composition_id); htsmsg_get_u32(sub, "composition_id", &composition_id);
htsmsg_get_u32(sub, "ancillary_id" , &ancillary_id); htsmsg_get_u32(sub, "ancillary_id" , &ancillary_id);
st.s->identifier = (composition_id & 0xffff) | ((ancillary_id & 0xffff) << 16); if(composition_id || ancillary_id)
{
st.s->ExtraData = new uint8_t[4];
st.s->ExtraSize = 4;
((uint16_t*)st.s->ExtraData)[0] = composition_id;
((uint16_t*)st.s->ExtraData)[1] = ancillary_id;
}
} else if(!strcmp(type, "TEXTSUB")) { } else if(!strcmp(type, "TEXTSUB")) {
st.s = new CDemuxStreamSubtitle(); st.s = new CDemuxStreamSubtitle();
st.s->codec = CODEC_ID_TEXT; st.s->codec = CODEC_ID_TEXT;
Expand Down
8 changes: 7 additions & 1 deletion xbmc/cores/dvdplayer/DVDDemuxers/DVDDemuxPVRClient.cpp
View file
Expand Up @@ -419,7 +419,13 @@ void CDVDDemuxPVRClient::RequestStreams()
{ {
st = new CDemuxStreamSubtitlePVRClient(this); st = new CDemuxStreamSubtitlePVRClient(this);
} }
st->identifier = props.stream[i].iIdentifier; if(props.stream[i].iIdentifier)
{
st->ExtraData = new uint8_t[4];
st->ExtraSize = 4;
((uint16_t*)st->ExtraData)[0] = (props.stream[i].iIdentifier >> 0) & 0xFFFFu;
((uint16_t*)st->ExtraData)[1] = (props.stream[i].iIdentifier >> 4) & 0xFFFFu;
}
m_streams[i] = st; m_streams[i] = st;
} }
else else
Expand Down
5 changes: 0 additions & 5 deletions xbmc/cores/dvdplayer/DVDStreamInfo.cpp
View file
Expand Up @@ -67,7 +67,6 @@ void CDVDStreamInfo::Clear()
bitrate = 0; bitrate = 0;
bitspersample = 0; bitspersample = 0;


identifier = 0;
orientation = 0; orientation = 0;
} }


Expand Down Expand Up @@ -108,7 +107,6 @@ bool CDVDStreamInfo::Equal(const CDVDStreamInfo& right, bool withextradata)
|| bitspersample != right.bitspersample ) return false; || bitspersample != right.bitspersample ) return false;


// SUBTITLE // SUBTITLE
if( identifier != right.identifier ) return false;


return true; return true;
} }
Expand Down Expand Up @@ -166,7 +164,6 @@ void CDVDStreamInfo::Assign(const CDVDStreamInfo& right, bool withextradata)
bitspersample = right.bitspersample; bitspersample = right.bitspersample;


// SUBTITLE // SUBTITLE
identifier = right.identifier;
} }


void CDVDStreamInfo::Assign(const CDemuxStream& right, bool withextradata) void CDVDStreamInfo::Assign(const CDemuxStream& right, bool withextradata)
Expand Down Expand Up @@ -211,7 +208,5 @@ void CDVDStreamInfo::Assign(const CDemuxStream& right, bool withextradata)
} }
else if( right.type == STREAM_SUBTITLE ) else if( right.type == STREAM_SUBTITLE )
{ {
const CDemuxStreamSubtitle *stream = static_cast<const CDemuxStreamSubtitle*>(&right);
identifier = stream->identifier;
} }
} }
1 change: 0 additions & 1 deletion xbmc/cores/dvdplayer/DVDStreamInfo.h
View file
Expand Up @@ -83,7 +83,6 @@ class CDVDStreamInfo
int bitspersample; int bitspersample;


// SUBTITLE // SUBTITLE
int identifier;


// CODEC EXTRADATA // CODEC EXTRADATA
void* extradata; // extra data for codec to use void* extradata; // extra data for codec to use
Expand Down

12 comments on commit 8967acd

@Jalle19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just spent an eternity bisecting and came to the conclusion that this is the commit that broke subtitles (both DVB and teletext) when watching live TV.

References:

http://forum.xbmc.org/showthread.php?tid=169386
http://trac.xbmc.org/ticket/14520

@elupus
Copy link
Contributor Author

@elupus elupus commented on 8967acd Dec 15, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you running external ffmpeg perhaps?

@Jalle19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'm not crazy :-) If it matters, I configure XBMC with this script:

#!/bin/sh

./configure --disable-joystick --disable-ssh --disable-nfs \
--disable-airplay --disable-upnp --disable-mysql --disable-optical-drive \
--disable-avahi --disable-samba

exit 0

@Jalle19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking it has something to do with how the stream identifier is split into a composition and ancillary ID, though I wasn't able to find any definite definition of how those values should be stored in the extradata field.

Edit: what I said was regarding DVDDemuxPVRClient, I guess DVDDemuxHTSP is what is used when watching live TV using pvr.hts.

@opdenkamp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DVDDemuxHTSP is not used by pvr.hts. it's used by our old htsp vfs handler. the add-on uses DVDDemuxPVRClient

@Jalle19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@opdenkamp: that explains a lot of things. Is anyone actually using that part of the code?

@opdenkamp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea, but it's been a candidate for my "to be removed" list for a while now but forgot to do it. feel free to nuke it

@elupus
Copy link
Contributor Author

@elupus elupus commented on 8967acd Dec 16, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please don't.. i use it. It's way easier and quicker to get setup for getting live tv.

@elupus
Copy link
Contributor Author

@elupus elupus commented on 8967acd Dec 16, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added my fix in: 27d94da

@opdenkamp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh you have just become the maintainer for that file @elupus ;-)

@iLLiac4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool does this mean that it is fixed now and it will be included in prebuild packages?

@Jalle19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iLLiac4: yes

Please sign in to comment.