Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit aeb572de authored by Sal Savage's avatar Sal Savage
Browse files

Remove cover art's dependency on external storage 

Recent changes to storage have caused unpredictable experiences with
cover art. Storage can take a long time to come up, or fail to come up
at all on multi-user builds. While the hope is these underlying problems
will be fixed, it's best for us now to remove this dependency all
together.

This change moves over the cover art storage mechanism from external
storage (where each image is a file) to internal hash map structures
(where each image is a Bitmap object). The provider has been updated
accordingly to be able to create ParcelFileDescriptor objects as
expected out of Bitmap objects.

Tag: #stability

Bug: 161396688
Test: atest BluetoothInstrumentationTests

Change-Id: I4f64375d4c0f9b083aadd741744379832da8bba2
Merged-In: I4f64375d4c0f9b083aadd741744379832da8bba2
parent 006ca63a

android/app/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtManager.java

+14 −5
Original line number Diff line number Diff line
@@ -18,6 +18,7 @@ package com.android.bluetooth.avrcpcontroller; 


import android.bluetooth.BluetoothDevice;

import android.bluetooth.BluetoothProfile;

import android.graphics.Bitmap;

import android.net.Uri;

import android.os.SystemProperties;

import android.util.Log;
@@ -77,8 +78,7 @@ public class AvrcpCoverArtManager { 
         * Notify of a get image download completing

         *

         * @param device The device the image handle belongs to

         * @param imageHandle The handle of the requested image

         * @param uri The Uri that the image is available at in storage

         * @param event The download event, containing the downloaded image's information

         */

        void onImageDownloadComplete(BluetoothDevice device, DownloadEvent event);

    }
@@ -193,7 +193,6 @@ public class AvrcpCoverArtManager { 
        for (BluetoothDevice device : mClients.keySet()) {

            disconnect(device);

        }

        mCoverArtStorage.clear();

    }



    /**
@@ -310,10 +309,20 @@ public class AvrcpCoverArtManager { 
    }



    /**

     * Remote a specific downloaded image if it exists

     * Get a specific downloaded image if it exists

     *

     * @param device The remote Bluetooth device associated with the image

     * @param imageUuid The UUID associated with the image you wish to retrieve

     */

    public Bitmap getImage(BluetoothDevice device, String imageUuid) {

        return mCoverArtStorage.getImage(device, imageUuid);

    }



    /**

     * Remove a specific downloaded image if it exists

     *

     * @param device The remote Bluetooth device associated with the image

     * @param imageHandle The handle associated with the image you wish to remove

     * @param imageUuid The UUID associated with the image you wish to remove

     */

    public void removeImage(BluetoothDevice device, String imageUuid) {

        mCoverArtStorage.removeImage(device, imageUuid);

android/app/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtProvider.java

+57 −18
Original line number Diff line number Diff line
@@ -21,12 +21,14 @@ import android.bluetooth.BluetoothDevice; 
import android.content.ContentProvider;

import android.content.ContentValues;

import android.database.Cursor;

import android.graphics.Bitmap;

import android.net.Uri;

import android.os.ParcelFileDescriptor;

import android.util.Log;



import java.io.File;

import java.io.FileNotFoundException;

import java.io.FileOutputStream;

import java.io.IOException;



/**

 * A provider of downloaded cover art images.
@@ -48,7 +50,6 @@ public class AvrcpCoverArtProvider extends ContentProvider { 
    private static final boolean DBG = Log.isLoggable(TAG, Log.DEBUG);



    private BluetoothAdapter mAdapter;

    private AvrcpCoverArtStorage mStorage;



    public AvrcpCoverArtProvider() {

    }
@@ -60,37 +61,69 @@ public class AvrcpCoverArtProvider extends ContentProvider { 
     * Get the Uri for a cover art image based on the device and image handle

     *

     * @param device The Bluetooth device from which an image originated

     * @param imageHandle The provided handle of the cover artwork

     * @param imageUuid The provided UUID of the cover artwork

     * @return The Uri this provider will store the downloaded image at

     */

    public static Uri getImageUri(BluetoothDevice device, String imageHandle) {

        if (device == null || imageHandle == null || "".equals(imageHandle)) return null;

    public static Uri getImageUri(BluetoothDevice device, String imageUuid) {

        if (device == null || imageUuid == null || "".equals(imageUuid)) return null;

        Uri uri = CONTENT_URI.buildUpon().appendQueryParameter("device", device.getAddress())

                .appendQueryParameter("handle", imageHandle)

                .appendQueryParameter("uuid", imageUuid)

                .build();

        debug("getImageUri -> " + uri.toString());

        return uri;

    }



    private ParcelFileDescriptor getImageDescriptor(BluetoothDevice device, String imageHandle)

            throws FileNotFoundException {

        debug("getImageDescriptor(" + device + ", " + imageHandle + ")");

        File file = mStorage.getImageFile(device, imageHandle);

        if (file == null) throw new FileNotFoundException();

        ParcelFileDescriptor pdf = ParcelFileDescriptor.open(file,

                ParcelFileDescriptor.MODE_READ_ONLY);

        return pdf;

    private Bitmap getImage(BluetoothDevice device, String imageUuid) {

        AvrcpControllerService service = AvrcpControllerService.getAvrcpControllerService();

        if (service == null) {

            debug("Failed to get service, cover art not available");

            return null;

        }



        AvrcpCoverArtManager manager = service.getCoverArtManager();

        if (manager == null) {

            debug("Failed to get cover art manager. Cover art may not be enabled.");

            return null;

        }

        return manager.getImage(device, imageUuid);

    }



    private ParcelFileDescriptor getImageDescriptor(BluetoothDevice device, String imageUuid)

            throws FileNotFoundException, IOException {

        debug("getImageDescriptor(" + device + ", " + imageUuid + ")");

        Bitmap image = getImage(device, imageUuid);

        if (image == null) {

            debug("Could not get requested image");

            throw new FileNotFoundException();

        }



        final ParcelFileDescriptor[] pipe = ParcelFileDescriptor.createPipe();

        Thread transferThread = new Thread() {

            public void run() {

                try {

                    FileOutputStream fout =

                            new ParcelFileDescriptor.AutoCloseOutputStream(pipe[1]);

                    image.compress(Bitmap.CompressFormat.PNG, 100, fout);

                    fout.flush();

                    fout.close();

                } catch (IOException e) {

                    /* Something bad must have happened writing the image data */

                }

            }

        };

        transferThread.start();

        return pipe[0];

    }



    @Override

    public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundException {

        debug("openFile(" + uri + ", '" + mode + "')");

        String address = null;

        String imageHandle = null;

        String imageUuid = null;

        BluetoothDevice device = null;

        try {

            address = uri.getQueryParameter("device");

            imageHandle = uri.getQueryParameter("handle");

            imageUuid = uri.getQueryParameter("uuid");

        } catch (NullPointerException e) {

            throw new FileNotFoundException();

        }
@@ -101,13 +134,19 @@ public class AvrcpCoverArtProvider extends ContentProvider { 
            throw new FileNotFoundException();

        }



        return getImageDescriptor(device, imageHandle);

        ParcelFileDescriptor pfd = null;

        try {

            pfd = getImageDescriptor(device, imageUuid);

        } catch (IOException e) {

            debug("Failed to create inputstream from Bitmap");

            throw new FileNotFoundException();

        }

        return pfd;

    }



    @Override

    public boolean onCreate() {

        mAdapter = BluetoothAdapter.getDefaultAdapter();

        mStorage = new AvrcpCoverArtStorage(getContext());

        return true;

    }

android/app/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtStorage.java

+64 −134
Original line number Diff line number Diff line
@@ -20,15 +20,13 @@ import android.bluetooth.BluetoothDevice; 
import android.content.Context;

import android.graphics.Bitmap;

import android.net.Uri;

import android.os.Environment;

import android.util.Log;



import java.io.File;

import java.io.FileOutputStream;

import java.io.IOException;

import java.util.Map;

import java.util.concurrent.ConcurrentHashMap;



/**

 * An abstraction of the file system storage of the downloaded cover art images.

 * An abstraction of the cover art image storage mechanism.

 */

public class AvrcpCoverArtStorage {

    private static final String TAG = "AvrcpCoverArtStorage";
@@ -36,6 +34,15 @@ public class AvrcpCoverArtStorage { 


    private final Context mContext;



    /* Each device gets its own place to land images. This makes it easier to clean things up on a

     * per device basis. This also allows us to be confident that acting on one device will not

     * impact the images of another.

     *

     * The "landing place" is simply a map that will direct a given UUID to the proper bitmap image

     */

    private final Map<BluetoothDevice, Map<String, Bitmap>> mDeviceImages =

            new ConcurrentHashMap<>(1);



    /**

     * Create and initialize this Cover Art storage interface

     */
@@ -47,94 +54,81 @@ public class AvrcpCoverArtStorage { 
     * Determine if an image already exists in storage

     *

     * @param device - The device the images was downloaded from

     * @param imageHandle - The handle that identifies the image

     * @param imageUuid - The UUID that identifies the image

     */

    public boolean doesImageExist(BluetoothDevice device, String imageHandle) {

        if (device == null || imageHandle == null || "".equals(imageHandle)) return false;

        String path = getImagePath(device, imageHandle);

        if (path == null) return false;

        File file = new File(path);

        return file.exists();

    public boolean doesImageExist(BluetoothDevice device, String imageUuid) {

        if (device == null || imageUuid == null || "".equals(imageUuid)) return false;

        Map<String, Bitmap> images = mDeviceImages.get(device);

        if (images == null) return false;

        return images.containsKey(imageUuid);

    }



    /**

     * Retrieve an image file from storage

     *

     * @param device - The device the images was downloaded from

     * @param imageHandle - The handle that identifies the image

     * @return A file descriptor for the image

     * @param imageUuid - The UUID that identifies the image

     * @return A Bitmap object of the image

     */

    public File getImageFile(BluetoothDevice device, String imageHandle) {

        if (device == null || imageHandle == null || "".equals(imageHandle)) return null;

        String path = getImagePath(device, imageHandle);

        if (path == null) return null;

        File file = new File(path);

        return file.exists() ? file : null;

    public Bitmap getImage(BluetoothDevice device, String imageUuid) {

        if (device == null || imageUuid == null || "".equals(imageUuid)) return null;

        Map<String, Bitmap> images = mDeviceImages.get(device);

        if (images == null) return null;

        return images.get(imageUuid);

    }



    /**

     * Add an image to storage

     *

     * @param device - The device the images was downloaded from

     * @param imageHandle - The handle that identifies the image

     * @param imageUuid - The UUID that identifies the image

     * @param image - The image

     */

    public Uri addImage(BluetoothDevice device, String imageHandle, Bitmap image) {

        debug("Storing image '" + imageHandle + "' from device " + device);

        if (device == null || imageHandle == null || "".equals(imageHandle) || image == null) {

    public Uri addImage(BluetoothDevice device, String imageUuid, Bitmap image) {

        debug("Storing image '" + imageUuid + "' from device " + device);

        if (device == null || imageUuid == null || "".equals(imageUuid) || image == null) {

            debug("Cannot store image. Improper aruguments");

            return null;

        }



        String path = getImagePath(device, imageHandle);

        if (path == null) {

            error("Cannot store image. Cannot provide a valid path to storage");

            return null;

        }



        try {

            String deviceDirectoryPath = getDevicePath(device);

            if (deviceDirectoryPath == null) {

                error("Cannot store image. Cannot get a valid path to per-device storage");

                return null;

            }

            File deviceDirectory = new File(deviceDirectoryPath);

            if (!deviceDirectory.exists()) {

                deviceDirectory.mkdirs();

        // A Thread safe way of creating a new UUID->Image set for a device. The putIfAbsent()

        // function will return the value of the key if it wasn't absent. If it returns null, then

        // there was no value there and we are to assume the reference we passed in was added.

        Map<String, Bitmap> newImageSet = new ConcurrentHashMap<String, Bitmap>(1);

        Map<String, Bitmap> images = mDeviceImages.putIfAbsent(device, newImageSet);

        if (images == null) {

            newImageSet.put(imageUuid, image);

        } else {

            images.put(imageUuid, image);

        }



            FileOutputStream outputStream = new FileOutputStream(path);

            image.compress(Bitmap.CompressFormat.PNG, 100, outputStream);

            outputStream.flush();

            outputStream.close();

        } catch (IOException e) {

            error("Failed to store '" + imageHandle + "' to '" + path + "'");

            return null;

        }

        Uri uri = AvrcpCoverArtProvider.getImageUri(device, imageHandle);

        Uri uri = AvrcpCoverArtProvider.getImageUri(device, imageUuid);

        mContext.getContentResolver().notifyChange(uri, null);

        debug("Image stored at '" + path + "'");

        debug("Image '" + imageUuid + "' stored for device '" + device.getAddress() + "'");

        return uri;

    }



    /**

     * Remove a specific image

     *

     * @param device The device you wish to have images removed for

     * @param imageHandle The handle that identifies the image to delete

     * @param device The device the image belongs to

     * @param imageUuid - The UUID that identifies the image

     */

    public void removeImage(BluetoothDevice device, String imageHandle) {

        debug("Removing image '" + imageHandle + "' from device " + device);

        if (device == null || imageHandle == null || "".equals(imageHandle)) return;

        String path = getImagePath(device, imageHandle);

        if (path == null) {

            error("Cannot remove image. Cannot get a valid path to storage");

    public void removeImage(BluetoothDevice device, String imageUuid) {

        debug("Removing image '" + imageUuid + "' from device " + device);

        if (device == null || imageUuid == null || "".equals(imageUuid)) return;



        Map<String, Bitmap> images = mDeviceImages.get(device);

        if (images == null) {

            return;

        }

        File file = new File(path);

        if (!file.exists()) return;

        file.delete();

        debug("Image deleted at '" + path + "'");



        images.remove(imageUuid);

        if (images.size() == 0) {

            mDeviceImages.remove(device);

        }



        debug("Image '" + imageUuid + "' removed for device '" + device.getAddress() + "'");

    }



    /**
@@ -145,91 +139,27 @@ public class AvrcpCoverArtStorage { 
    public void removeImagesForDevice(BluetoothDevice device) {

        if (device == null) return;

        debug("Remove cover art for device " + device.getAddress());

        String deviceDirectoryPath = getDevicePath(device);

        if (deviceDirectoryPath == null) {

            error("Cannot remove images for device. Cannot get a valid path to storage");

            return;

        }

        File deviceDirectory = new File(deviceDirectoryPath);

        deleteStorageDirectory(deviceDirectory);

        mDeviceImages.remove(device);

    }



    /**

     * Clear the entirety of storage

     */

    public void clear() {

        String storageDirectoryPath = getStorageDirectory();

        if (storageDirectoryPath == null) {

            error("Cannot remove images, cannot get a valid path to storage. Is it mounted?");

            return;

        }

        File storageDirectory = new File(storageDirectoryPath);

        deleteStorageDirectory(storageDirectory);

    }



    private String getStorageDirectory() {

        String dir = null;

        if (Environment.MEDIA_MOUNTED.equals(Environment.getExternalStorageState())) {

            dir = mContext.getExternalFilesDir(null).getAbsolutePath() + "/coverart";

        } else {

            error("Cannot get storage directory, state=" + Environment.getExternalStorageState());

        }

        return dir;

    }



    private String getDevicePath(BluetoothDevice device) {

        String storageDir = getStorageDirectory();

        if (storageDir == null) return null;

        return storageDir + "/" + device.getAddress().replace(":", "");

    }



    private String getImagePath(BluetoothDevice device, String imageHandle) {

        String deviceDir = getDevicePath(device);

        if (deviceDir == null) return null;

        return deviceDir + "/" + imageHandle + ".png";

    }



    private void deleteStorageDirectory(File directory) {

        if (directory == null) {

            error("Cannot delete directory, file is null");

            return;

        }

        if (!directory.exists()) return;

        File[] files = directory.listFiles();

        if (files == null) {

            return;

        }

        for (int i = 0; i < files.length; i++) {

            debug("Deleting " + files[i].getAbsolutePath());

            if (files[i].isDirectory()) {

                deleteStorageDirectory(files[i]);

            } else {

                files[i].delete();

            }

        }

        directory.delete();

        debug("Clearing all images");

        mDeviceImages.clear();

    }



    @Override

    public String toString() {

        String s = "CoverArtStorage:\n";

        String storageDirectory = getStorageDirectory();

        s += "    Storage Directory: " + storageDirectory + "\n";

        if (storageDirectory == null) {

            return s;

        }



        File storage = new File(storageDirectory);

        File[] devices = storage.listFiles();

        if (devices != null) {

            for (File deviceDirectory : devices) {

                s += "    " + deviceDirectory.getName() + ":\n";

                File[] images = deviceDirectory.listFiles();

                if (images == null) continue;

                for (File image : images) {

                    s += "      " + image.getName() + "\n";

                }

        for (BluetoothDevice device : mDeviceImages.keySet()) {

            Map<String, Bitmap> images = mDeviceImages.get(device);

            s += "  " + device.getAddress() + " (" + images.size() + "):";

            for (String uuid : images.keySet()) {

                s += "\n    " + uuid;

            }

            s += "\n";

        }

        return s;

    }

android/app/tests/unit/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtStorageTest.java

+10 −12
Original line number Diff line number Diff line
@@ -34,7 +34,6 @@ import org.junit.Before; 
import org.junit.Test;

import org.junit.runner.RunWith;



import java.io.File;

import java.io.InputStream;



/**
@@ -89,9 +88,8 @@ public final class AvrcpCoverArtStorageTest { 
    }



    private void assertImageSame(Bitmap expected, BluetoothDevice device, String handle) {

        File file = mAvrcpCoverArtStorage.getImageFile(device, handle);

        Bitmap fromStorage = BitmapFactory.decodeFile(file.getPath());

        Assert.assertTrue(expected.sameAs(fromStorage));

        Bitmap image = mAvrcpCoverArtStorage.getImage(device, handle);

        Assert.assertTrue(expected.sameAs(image));

    }



    @Test
@@ -203,29 +201,29 @@ public final class AvrcpCoverArtStorageTest { 
    @Test

    public void getImageThatDoesntExist_returnsNull() {

        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle1));

        File file = mAvrcpCoverArtStorage.getImageFile(mDevice1, mHandle1);

        Assert.assertEquals(null, file);

        Bitmap image = mAvrcpCoverArtStorage.getImage(mDevice1, mHandle1);

        Assert.assertEquals(null, image);

    }



    @Test

    public void getImageNullDevice_returnsNull() {

        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle1));

        File file = mAvrcpCoverArtStorage.getImageFile(null, mHandle1);

        Assert.assertEquals(null, file);

        Bitmap image = mAvrcpCoverArtStorage.getImage(null, mHandle1);

        Assert.assertEquals(null, image);

    }



    @Test

    public void getImageNullHandle_returnsNull() {

        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle1));

        File file = mAvrcpCoverArtStorage.getImageFile(mDevice1, null);

        Assert.assertEquals(null, file);

        Bitmap image = mAvrcpCoverArtStorage.getImage(mDevice1, null);

        Assert.assertEquals(null, image);

    }



    @Test

    public void getImageEmptyHandle_returnsNull() {

        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle1));

        File file = mAvrcpCoverArtStorage.getImageFile(mDevice1, "");

        Assert.assertEquals(null, file);

        Bitmap image = mAvrcpCoverArtStorage.getImage(mDevice1, "");

        Assert.assertEquals(null, image);

    }



    @Test