Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

GUI_MSG_UPDATE_ITEM occasionally not working correctly #955

Merged
merged 1 commit into from

2 participants

Michal Piechowiak jmarshallnz
Michal Piechowiak
Collaborator

GUI_MSG_UPDATE_ITEM msg from https://github.com/xbmc/xbmc/blob/master/xbmc/utils/SaveFileStateJob.h#L92 will not work correctly when old item in items vector has startOffset set to STARTOFFSET_RESUME and new one has set it 0 (it won't update item because CFileItem::IsSamePath will return false).

This is more request for comments than pull request (my commit is just dummy fix that work in my case, but propably will fail somewhere else). We could have flag to determine if we need to compare startoffset to differentiate items - f.e. add some property to fileitems when creating them from cuesheet (and anything similiar). Any thoughts on this?

jmarshallnz
Owner

Yeah - agreed that we need a way to differentiate items. I'm not sure the best technique for cue sheet items, but clearly using a separate value that's also used by something else (start offset) is not working. There was some comments on this in the cue sheet stuff in #840. Essentially to give a separate match path for those items that are actually differentiated but otherwise have the correct path.

Michal Piechowiak
Collaborator

So set path to f.e. "path_to_file.ext?startOffset=XXX" and when we start playback strip off additional params and read startOffset? Wasn't yet looking at code at all, just wondering what needs to be done.

jmarshallnz
Owner

I'm not sure. I suspect that will cause issues all over the show, however. As far as I'm aware, cue sheet handling is OK, as-is, so maybe just have m_matchPath be used in IsSamePath() and have cue sheet items set that? Just have to make sure it's set everywhere it needs to be (Cue stuff + musicdb of cue's most likely). This way we have a well-defined member that's used just for matching items and nothing else, so that other uses of m_lStartOffset don't collide.

Deleted user

how about we simply set a prop ?

Michal Piechowiak
Collaborator

@cptspiff
I started doing it this way (https://gist.github.com/c99dfbbc15df4e7456b9) but as I didn't have neither handy cue stuff nor more time to spend on it I don't know if it's actually working or where I would need to make more changes (hints always welcomed). This is yet another postponed work from me waiting for my "free time and will"... TBH after pike's complains about cue "shit", I'm affraid to dl any samples and play with them ;)

Michal Piechowiak pieh compare dedicated item_start property instead of multi purpose starto…
…ffset member to determine if FileItem represents same item
2b74a4d
Michal Piechowiak
Collaborator

This is aproach of setting dedicated "item_start" property to differentiate items (well, songs now as we don't support videos) in single file, seems to be working fine (as in no regressions for #840). Tested with "1 large audio file album + 1 .cue with indexes" both from file mode and from library.

jmarshallnz
Owner

It's fine, though I wonder whether having a more generic compare path property might be more useful - more of a PITA to set up though if you use a property rather than a member as you need to format it up all over the show - maybe best just to leave it as this for now and we can extend later if needed.

Feel free to pull.

Michal Piechowiak pieh merged commit 12f23d2 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 15, 2012
  1. Michal Piechowiak

    compare dedicated item_start property instead of multi purpose starto…

    pieh authored
    …ffset member to determine if FileItem represents same item
This page is out of date. Refresh to see the latest.
Showing with 12 additions and 5 deletions.
  1. +11 −5 xbmc/FileItem.cpp
  2. +1 −0  xbmc/music/MusicDatabase.cpp
16 xbmc/FileItem.cpp
View
@@ -74,6 +74,7 @@ CFileItem::CFileItem(const CSong& song)
m_strPath = song.strFileName;
GetMusicInfoTag()->SetSong(song);
m_lStartOffset = song.iStartOffset;
+ SetProperty("item_start", song.iStartOffset);
m_lEndOffset = song.iEndOffset;
m_strThumbnailImage = song.strThumb;
}
@@ -1148,29 +1149,34 @@ bool CFileItem::IsSamePath(const CFileItem *item) const
if (!item)
return false;
- if (item->GetPath() == m_strPath && item->m_lStartOffset == m_lStartOffset) return true;
+ if (item->GetPath() == m_strPath)
+ {
+ if (item->HasProperty("item_start") || HasProperty("item_start"))
+ return (item->GetProperty("item_start") == GetProperty("item_start"));
+ return true;
+ }
if (IsMusicDb() && HasMusicInfoTag())
{
CFileItem dbItem(m_musicInfoTag->GetURL(), false);
- dbItem.m_lStartOffset = m_lStartOffset;
+ dbItem.SetProperty("item_start", GetProperty("item_start"));
return dbItem.IsSamePath(item);
}
if (IsVideoDb() && HasVideoInfoTag())
{
CFileItem dbItem(m_videoInfoTag->m_strFileNameAndPath, false);
- dbItem.m_lStartOffset = m_lStartOffset;
+ dbItem.SetProperty("item_start", GetProperty("item_start"));
return dbItem.IsSamePath(item);
}
if (item->IsMusicDb() && item->HasMusicInfoTag())
{
CFileItem dbItem(item->m_musicInfoTag->GetURL(), false);
- dbItem.m_lStartOffset = item->m_lStartOffset;
+ dbItem.SetProperty("item_start", item->GetProperty("item_start"));
return IsSamePath(&dbItem);
}
if (item->IsVideoDb() && item->HasVideoInfoTag())
{
CFileItem dbItem(item->m_videoInfoTag->m_strFileNameAndPath, false);
- dbItem.m_lStartOffset = item->m_lStartOffset;
+ dbItem.SetProperty("item_start", item->GetProperty("item_start"));
return IsSamePath(&dbItem);
}
if (HasProperty("original_listitem_url"))
1  xbmc/music/MusicDatabase.cpp
View
@@ -870,6 +870,7 @@ void CMusicDatabase::GetFileItemFromDataset(const dbiplus::sql_record* const rec
item->GetMusicInfoTag()->SetTitle(record->at(song_strTitle).get_asString());
item->SetLabel(record->at(song_strTitle).get_asString());
item->m_lStartOffset = record->at(song_iStartOffset).get_asInt();
+ item->SetProperty("item_start", item->m_lStartOffset);
item->m_lEndOffset = record->at(song_iEndOffset).get_asInt();
item->GetMusicInfoTag()->SetMusicBrainzTrackID(record->at(song_strMusicBrainzTrackID).get_asString());
item->GetMusicInfoTag()->SetMusicBrainzArtistID(record->at(song_strMusicBrainzArtistID).get_asString());
Something went wrong with that request. Please try again.