Page 1 of 1

1664 Does not work with d_itunes4

Posted: Tue Oct 22, 2013 1:53 pm
by markstuartwalker
I am sorry not to have reported this sooner but the 4.1 betas do not work with my d_itunes plugin.

Please PM me with any details of what might have changed in 4.1 so that I can help resolve this matter.

Re: 1664 Does not work with d_itunes4

Posted: Wed Oct 23, 2013 7:06 am
by Ludek
Thank you for reporting, looking into it: http://www.ventismedia.com/mantis/view.php?id=11406

Re: 1664 Does not work with d_itunes4

Posted: Thu Oct 24, 2013 3:30 am
by markstuartwalker
@Ludek

Can you explain "Neverthless there is still an issue, it seems that DEVICE_NotifyPlaylist in d_iTunes4.dll damages MM memory structures and potentional AV appears.
I workarounded it, but this should be fixed by the plugin developer."?

Am I overrunning some memory space?

Re: 1664 Does not work with d_itunes4

Posted: Thu Oct 24, 2013 8:53 am
by Ludek
While I was fixing the issue, calling the DEVICE_NotifyPlaylist caused a memory damage, because some my structures were corrupted after performing the DEVICE_NotifyPlaylist

I workarounded it by caching the needed memory, but it seems that there is something wrong. I now that the first parameter is pointer (device handle), maybe you are working with it somehow wrong? I haven't seen your source code for DEVICE_NotifyPlaylist, if I could see it, I could say more probably. You might want to PM me the sources.

Otherwise the others issues are fixed in 1668 that is going to be posted before weekend if everything goes well.

Re: 1664 Does not work with d_itunes4

Posted: Thu Oct 24, 2013 10:06 am
by markstuartwalker
I don't even get that far. I get an exception thrown the instant that I click "sync"

"Project Mediamonkey raised an exception class Econvertor "" is not a valid integer.

Has there been a change to any of the method signatures?

Re: 1664 Does not work with d_itunes4

Posted: Thu Oct 24, 2013 10:29 am
by Ludek
markstuartwalker wrote: "Project Mediamonkey raised an exception class Econvertor "" is not a valid integer.
This is fixed in 1668.
markstuartwalker wrote: Has there been a change to any of the method signatures?
No, the method interface is still the same as in MM 4.0.
I guess that in 4.0 the memory corruption was too, but it was silent because I haven't access the corrupted memory anymore later.
Is the plugin written in Delphi or in C++ ?

Re: 1664 Does not work with d_itunes4

Posted: Thu Oct 24, 2013 3:50 pm
by markstuartwalker
DElphi

Re: 1664 Does not work with d_itunes4

Posted: Sat Oct 26, 2013 11:58 am
by markstuartwalker
I'm not sure what you might make of it ...

Code: Select all

procedure DEVICE_NotifyPlaylist( DeviceHandle : integer; M3UFileName, M3UTitle : PWideChar;
              TrackCount : integer; Tracks : PWideCharArr); stdcall;
begin
  log.text(3,'DEVICE_NotifyPlaylist start',M3UTitle,TrackCount);
  try
    DeviceiTunesCreatePlaylist( DeviceHandle , M3UFileName, M3UTitle, TrackCount, Tracks);
  except
    on e: exception do
    begin
      log.exception('DEVICE_NotifyPlaylist', e);
      DeviceInterf.TerminateThreads(DeviceHandle);
    end;
  end;
  log.text(3,'DEVICE_NotifyPlaylist end');
end;



/ create playlists
// this provides a playlist name and an array of tracknames
procedure DeviceiTunesCreatePlaylist(DeviceHandle :Integer ; M3UFileName: PWideChar; M3UTitle: PWideChar;
  mmTrackCount: integer; mmTracks: PWideCharArr);
var
  pl: iituserplaylist;
//  upl: iituserplaylist;
  v1,v2: oleVariant;
  i,j,firstDifferent: integer;
  id: integer;
  FQN, DeviceTrackPath: WideString;
  itTrack: IITTrack;
  itTrackCached: TitTrack;
  myApp: IiTunes;
  dev:TDevice;
  run:Boolean;
begin
  log.text(2, 'iTunesCreatePlaylist', M3UTitle);

  dev:=DeviceList.getByHandle(DeviceHandle);
  if dev=nil then exit;
  run:=true;

  // take this out of the delete list
  // s othat it does not get deleted later
  removeFromDeleteList( dev,M3UTitle ) ;

  case dev.Config.iTunesMode of
  0:
    begin
      if not dev.g.iTunesRunning then run:=false;
      myApp := getiTunesApp();
      if myApp=nil then run:=false
    end;
  end;

  if run then
  begin
    // determine it's full name
    FQN := FQname(M3UTitle,dev.Config);

    log.text(2, 'Playlist FQN', FQN);

    if dev.g.BlackList.onBlacklist(FQN) then
      log.text(1, 'Blacklisted doNotCreatePlaylistsFor',dev.Config.doNotCreatePlaylistsFor, FQN)
    else
    begin

      // dump to a text file
      case dev.Config.iTunesMode of
      0:
      begin
      // reuse an existing one or create a new one
      pl := reuseOrCreatePlaylist(dev,myApp, FQN,dev.config) as IITUserPlaylist;
  //      upl:=pl as IItUserPlaylist;

      // only bother if the lists are different lengths or tracks
  //      if dev.Config.checkPlaylistsWithSameLength or ( mmTrackCount <> pl.Tracks.count ) then
      if differentPlaylists(myApp, pl, mmTrackCount, mmTracks) then
      begin
        log.text(2, FQN + ' different lengths (MM,iT)', mmTrackCount,
          pl.Tracks.count);

        // remove any tracks which are differnt
        // returns the number of the first different track
        firstDifferent:=findFirstDifferentTrack(myApp, pl, mmTrackCount, mmTracks);

        // remove all the tracks  in the iTunes playlist that we don't need
        deleteItPlaylistTracks(pl,firstDifferent,pl.tracks.count);

        // and append the outstanding ones to the end of the remaining list
        // from the MM playlist to the iTunes playlist
        for i := firstDifferent to mmTrackCount do
        begin
          // read the combined highlow name from the MM list
          DeviceTrackPath := mmTracks[i-1];
          log.text(2, 'Track name', DeviceTrackPath);

          // look it up in the cached iTunes library?
          id:=DeviceTrackPathToID(DeviceTrackPath);
          itTrackCached :=dev.g.TrackCache.getByTrackID(id);
          if itTrackCached <> nil then
          begin
            itTrack := itTrackCached.itPointer(myApp);

            if itTrack <> nil then
            begin

              for j := 0 to 9 do
              begin
                // append an existing track to the end of the playlist's list
                log.text(2, 'Add track to playlist by ID', DeviceTrackPath);
                try
                  v1 := pl;
                  v2 := itTrack ;
                  v1.AddTrack(v2);
                  // if we got this far then we've succeeded
                  break ;
                except
                  on e: exception do
                    log.exception( pl.Name + ' AddTrack(itTrack) ' + inttostr(j) + ' ' + DeviceTrackPath, e);
                end;
                sleep(100);
              end;
            end
            else
            begin
              // SHOULD NEVER HAPPEN, ALL FILES SHOULD BE ADDED BY THIS POINT
              log.exception('Track='+itTrackCached.name+' not available in iTunes to add to playlist='+FQN+' '+DeviceTrackPath ,nil);
            end;
          end;
        end;

        // and shuffle them
        if dev.Config.setAllPlaylistsToShuffle then
          pl.Shuffle := true;
      end
      else
        log.text(2, M3UTitle + ' same lengths ' + inttostr(mmTrackCount), '(skipped)');
      end;
      1:
      begin
        // don't be clever - simply re-create it every time
        createM3Ufile(dev.g.TrackCache,
          mmTrackCount,
          mmTracks,
          makeCachePlaylistFilename(dev.Config.cacheFolder,safeFilename(FQN)), //M3UTitle),
          (dev.Config.iTunesMode=1));

      end;
      end;

    end;
  end;

end;

Re: 1664 Does not work with d_itunes4

Posted: Sun Oct 27, 2013 12:24 pm
by markstuartwalker
Please take a look at this. I have never been confident about this piece of code as I never so any allocation of extra space to hold the strings. Might this be the source of memory corruptions?

Code: Select all

// change the file names to be 'device names'
// in this implementation the name is the MM ID number
// suffixed by the track title.
procedure DeviceiTunesFixFilenames(DeviceHandle :Integer ; filenamecount: integer;
  filenamelist: PFileNameRecArr);
var
  i: integer;
  filename: WideString;
  mmTrackID: integer;
  itTrack: TitTrack;
  rec: PFileNameRec;
  dev:TDevice;
begin

  log.text(0, 'iTunesFixFilenames', filenamecount);

  // context
  dev:=DeviceList.getByHandle(DeviceHandle);
  if dev=nil then exit;

  rec := @filenamelist[0];
  for i := 0 to filenamecount - 1 do
  begin
    // have we got an existing filename in the MM database (by ID)?
    mmTrackID := rec.track_id;
    filename := IDtoDeviceTrackPath(rec.filename,mmTrackID);

    // is this item on the device already?
    itTrack := dev.g.TrackCache.getByTrackID(mmTrackID);
    if (itTrack <> nil) then
    begin
      // yes, so we know what the name will be (it's already there);
      log.text(2, i, 'track already in iTunes', filename);
      Inc(stat.existing);
    end
    else
    begin
      // so it's a new file to be added
      log.text(2, i, 'new track to be added', filename);
      Inc(stat.new);
    end;

    StrCopy(rec.filename, PWideChar(filename));
    Inc(rec);
    Inc(stat.filenames);
  end;

  log.text(0, 'iTunesFixFilenames done');

end;

Code: Select all

  PWideCharArr = ^TWideCharArr;
  TWideCharArr = array [0..MAXINT div 4-1] of PWideChar;

  PFileNameRec = ^TFileNameRec;
  TFileNameRec = record
    filename: PWideChar;    // at least 256 chars wide buffer for filename
    track_id: integer;      // track id as in MM.DB file
    track_cookie: cardinal; // COM object SDBSongData can be accessed this way
    flags: integer;         // Not used at this moment
  end;

  PFileNameRecArr = ^TFileNameRecArr;
  TFileNameRecArr = array [0..MAXINT div 32-1] of TFileNameRec;

  PScanRecord = ^TScanRecord;
//  TScanRecord = record
//    filename: PWideChar;
//    filesize: int64;
//    lastmodified: TDateTime;      // i.e. a float
//  end;
  TScanRecord = record
    filename: PWideChar;
    filesize: int64;
    lastmodified: TDateTime; // i.e. a float
    track_cookie: cardinal; // COM object SDBSongData can be accessed this way
  end;

  PScanResultArr = ^TScanResultArr;
  TScanResultArr = array [0..MAXINT div 32-1] of PScanRecord;

  PScanResult = ^TScanResult;
  TScanResult = record
    FileCount: integer;
    Files: PScanResultArr;
  end;

Re: 1664 Does not work with d_itunes4

Posted: Mon Oct 28, 2013 6:32 am
by markstuartwalker
I have worked out what the problem is. It is a regression of a problem found when MM4 was in beta.

MM4 introduced a concept of "background scanning" which was to make things work quicker. My plugin cannot work with this as the DEVICE_ListContent function must be called initially so that iTunes can be scanned and the contents cached to do the sync later.

What is happening with MM4.1 is that (once again) the DEVICE_ListContent function is not being called. The resulting memory corruptions are all a result of uninitialised pointers and caches. I have defensively coded around the majority of these to protect the exceptions.

So, it would appear that you haven't changed the API method signatures but you have changed the order of the DEVICE_* calls.

Update: I have tried 1668 and SUDDENLY IT ALL WORKED.

I have added many additional log updates and made the operation robust to handle unexpected nil pointers. I have corrected no bugs in my code.

Clearly we need to pursue this memory corruption that you are reporting....

Re: 1664 Does not work with d_itunes4

Posted: Tue Oct 29, 2013 8:37 am
by Ludek
markstuartwalker wrote: So, it would appear that you haven't changed the API method signatures but you have changed the order of the DEVICE_* calls.
I have checked the changes and there doesn't seem to be any change to the mentioned structures and interfaces.
You are true that we added several new DEVICE_* calls (like DEVICE_SetString), but this shouldn't be an issue, because we call them only when they are included within the DLL interface. Also its order is not relevant as we use GetProcAddress( DEVICE_*)
markstuartwalker wrote: Update: I have tried 1668 and SUDDENLY IT ALL WORKED.
Yes, 1668 works, but I still believe there is a hidden problem. I could create a special MediaMonkey.exe without the workaround so that you could simulate it.

Re: 1664 Does not work with d_itunes4

Posted: Tue Oct 29, 2013 8:48 am
by Ludek
Actually I think I see it, your interface is

Code: Select all

procedure DEVICE_NotifyPlaylist( DeviceHandle : integer; M3UFileName, M3UTitle : PWideChar;
              TrackCount : integer; Tracks : PWideCharArr); stdcall;
while MM interface is:

Code: Select all

procedure DEVICE_NotifyPlaylist( DeviceHandle : integer; M3UFileName, M3UTitle : PWideChar;
              TrackCount : integer; Tracks : PWideCharArr; M3UPath: PWideChar); stdcall;
Looking into SVN the interface was changed in course of http://www.ventismedia.com/mantis/view.php?id=6255
i.e. it was presented in MM 4.0 too, but the error was hidden.

Re: 1664 Does not work with d_itunes4

Posted: Wed Oct 30, 2013 3:44 am
by markstuartwalker
Right! At last, something tangible.

I have modified my code and retested with 1668 and observed no change. I will PM you with a location that you can download the fixed dll for testing.

Update: I have also noticed that the pop-up that I used to get in Delphi Studio as MM ended saying "MM has terminated badly" has stopped appearing. I do wonder if this has been causing random crashes and misbehaves for a long time. This function will be called once per playlist each time it is run, causing stack corruption each time.

Re: 1664 Does not work with d_itunes4

Posted: Wed Oct 30, 2013 10:31 am
by Ludek
Yes, it works fine with the updated d_itunes4.dll that you posted me via PM ;-)
The stack corruption has gone.

So I removed the workaround in build 1669, please post the update here: http://www.mediamonkey.com/addons/brows ... or-itunes/

Re: 1664 Does not work with d_itunes4

Posted: Wed Oct 30, 2013 3:10 pm
by markstuartwalker
New version uploaded