dBpa PropertyHandler/IPropertyStore misbehaviour.

[LeoDavidson]

Hi,

I think I have found a problem with the PropertyHandler which dBpoweramp installs. This is causing a knock-on problem in another app that I've been investigating.

• Background:
  
  The other app is a file manager which includes a metadata/attributes editor. When updating a file's properties, the app first tries the installed IPropertyStore handler, if any. Then, for any remaining properties which the handler rejected, it falls back on a custom storage method.
  
  This way the properties are stored in a way that everything on the system can see where possible, and where not possible we still allow people to use the full set of properties within our app.
  
  
• What goes wrong:
  
  With dbPa installed, the IPropertyStore we get for .MP3 files succeeds when we do a SetValue on the Keywords property. (If dbPa is not installed, the default handler returns STG_E_ACCESSDENIED here and we fall-back on our custom storage method.)
  
  After dbPa's handler accepts the value, calling Commit to save the change also returns success.
  
  However, if we then create a new IPropertyStore for the updated file and use GetValue on the same property, an empty property value is returned instead of the value we had just saved.
  
  (If we try calling GetValue on the original IPropertyStore instance, before calling commit, the proper value is returned. So it seems like the SetValue method is keeping the new data in memory, but it is not being correctly saved to disk (and/or loaded from disk). I don't know if the bug is that SetValue accepts a property it shouldn't or that the load/save code doesn't persist the value properly. From our point of view, fixing either would make things work.)
  
  
• Test exe and source code
  
  I've written a minimal command-line tool which can be used to reproduce the problem. VS2008 C++ source code, and compiled x86 and x64 exes, are here:
  
  

  http://leo.dopus.com/temp/dBpaPropBug.zip

  
  
  The exes require Vista or above.
  
  If you are on 64-bit Windows, use the 64-bit exe. (It seems on a 64-bit OS dBpa only installs a 64-bit PropertyHandler, leaving 32-bit exes unaffected.)
  
  Run the appropriate exe from a command-line with the full path (not just the name) of an MP3 file as the only argument.
  
  With dBpa installed it will print something like this:
  
  

  > dBpaPropBug_64bit.exe C:\Test\15-Silent.mp3
  File "C:\Test\15-Silent.mp3"
  GetValue 1 type: 4127
  GetValue 2 type: 0

  (The first GetValue reports the PropVariant type returned by the first handler instance before the commit. The second GetValue reports the type returned by the second handler. As you can see, it becomes 0 i.e. VT_EMPTY.)
  
  With dBpa not installed (or just reset HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows\Curr entVersion\PropertySystem\PropertyHandlers\.mp3 to normal):
  
  

  > dBpaPropBug_64bit.exe C:\Test\15-Silent.mp3
  File "C:\Test\15-Silent.mp3"
  SetValue 1 fail: 80030005
  GetValue 2 type: 0

I hope I've provided the right level of detail. Let me know if you need anything clarified or if it looks like we're doing something wrong in our code.

Cheers,
Leo

[Spoon]

Re: dBpa PropertyHandler/IPropertyStore misbehaviour.

After calling Commit, see if the file time has changed on the file.

[LeoDavidson]

Re: dBpa PropertyHandler/IPropertyStore misbehaviour.

What if some properties are saved but not others?

Or if some property handlers either preserve the file timestamp or store the data in a way that doesn't change it in the first place (e.g. separate from the file itself)?

This is a bug, isn't it? Your handler should not be accepting a property it cannot save and/or returning success on a failed commit.

[Spoon]

Re: dBpa PropertyHandler/IPropertyStore misbehaviour.

Everything can be saved to mp3 tags (as long as it is not id3v1), anything which does not fit the standard placeholders goes to the TXXX comment tag (off the top of my head).

[LeoDavidson]

Re: dBpa PropertyHandler/IPropertyStore misbehaviour.

I checked the file date, size and binary contents and it's not being modified at all.

I don't know if it's a factor, but the property value is a single-string array of strings (VT_VECTOR | VT_LPWSTR).

PROPERTYKEY pk;
pk.fmtid = FMTID_SummaryInformation;
pk.pid   = PIDSI_KEYWORDS;

const wchar_t *aszTagValues[] = { L"MooTag1" };

PROPVARIANT propvar;
InitPropVariantFromStringVector(aszTagValues, _countof(aszTagValues), &propvar);

hr = pStore->SetValue(pk, propvar);

[dbfan]

Re: dBpa PropertyHandler/IPropertyStore misbehaviour.

You would knave to do it the exact same way windows does it, which I think is one at a time.