Open callback missing "context" argument

[geiseri]

I am trying to integrate the encoding to use a C++ class so I need access to the pData field of the PNGFILE as to access my file IO operations. All of the other methods include that structure save for the open callback. I saw it returned a void * but that seems to be the actual handle for the opened file and not the PNGFILE structure. Could that argument be added? Or am I using this incorrectly?

[bitbank2]

It sounds like you're not using it correctly. I provide the encoder as C89 code. I also provide an Arduino C++ wrapper. If you're writing your own C++ class, then why do you need my open callback? You're either providing a buffer large enough to hold the entire output data or you provide a write() callback to handle the data as it's generated.

[geiseri]

Oh, sorry I was confused between what was going. I had meant fHandle not pData, that would definitely cause some entertaining results =)

In my case I am using the IDF since I hate Arduino's rampant use of globals. The class in question is generating a FFT and dumping it to the image. Right now I am using a buffer and just using open(uint8_t *pOutput, int iBufferSize). My issue is that I am running out of memory because of other sensor data in the system. I was thinking that I would be able to use the file api instead so I could get rid of the need to store the buffer. Most of the time when I am dealing with a C api I use a pattern of making a "context" struct that holds any callbacks I need as a std::function<...> and then have it dispatch lambdas. Then pass that in as user data for the callback. I was hoping to do something like this:

namespace detail {
  struct PngContext {
    std::function<void* (const char*)> open_callback;
    std::function<void()> close_callback;
    std::function<int32_t(int32_t)> seek_callback;
    std::function<int32_t(uint8_t*, int32_t)> read_callback;
    std::function<int32_t(uint8_t*, int32_t)> write_callback;
  };

  static  void* open_callback_c(const char*) {
????
  }
  static void close_callback_c(PNGFILE* png) {
    auto self = (PngContext*) png->fHandle;
    self->close_callback();
  }
  static int32_t seek_callback_c(PNGFILE* png, int32_t pos) {
    auto self = (PngContext*) png->fHandle;
    return self->seek_callback(pos);
  }
  static int32_t read_callback_c(PNGFILE* png, uint8_t* buff, int32_t len) {
    auto self = (PngContext*) png->fHandle;
    return self->read_callback(buff, len);
  }
  static int32_t write_callback_c(PNGFILE* png, uint8_t* buff, int32_t len) {
    auto self = (PngContext*) png->fHandle;
    return self->write_callback(buff, len);
  }
}

void save_png(int rows) {
  auto png = new PNG();
  ....
  detail::PngContext ctx{
    [](const char* szFilename) { ...},
    []() {...},
    [](int32_t iPosition) {...},
    [](uint8_t* pBuf, int32_t iLen) {... },
    [](uint8_t* pBuf, int32_t iLen) {...}
  };

  auto rc = png->open("file.png", 
    &detail::open_callback_c,
    &detail::close_callback_c,
    &detail::read_callback_c,
    &detail::write_callback_c,
    &detail::seek_callback_c);
  for (size_t row = 0; row < rows && rc == PNG_SUCCESS; row++) {
    rc = png->addLine(get_stride(row));
  }
}

Does that make more sense?

[bitbank2]

I think I understand what you're trying to do. The current code supports it. Return your lambda structure from the open call and then it will be passed to the write callback as a member of the PNGFILE structure (fHandle).

[geiseri]

Sadly it can't as C callbacks must be global statics and cannot be class members.

[bitbank2]

You lost me. Class members of what? I was talking about structure members. This design works for everyone else; you're the first to have an issue with it. Like I said, the Arduino C++ class is a thin wrapper on top of the C code doing the actual work. Replace the Arduino library class with your own. It probably involves less than 30 lines of code.

[geiseri]

The lifetime of the PNG object is local to the save_png function. Technically it's a method on a class, but just have is separate to keep the example short. My normal strategy when dealing with C callbacks is to create some sort of struct that will hold instances of the lambdas I wish to use (in my case detail::PngContext), then pass that instance as the "user data" pointer that C callbacks usually use to handle state , and use that to dispatch back to the C++ caller. This way I can save the need for any global state. Does that make more sense?

In the case of PNG::open my understanding is that whatever is returned from thePNG_OPEN_CALLBACK is placed into fHandle for use in the other callbacks. So intent is that it would allocate the object that controls the file io operations. Is this a correct understanding?

If that is the intent where I get stuck is that I can't allocate the detail::PngContext inside of that function since it would not be able to capture the data from the save_png function scope that is needed for the lambdas.

That being said, I think what I am trying to do is incompatible with the current design, so I need to rethink things. Up until now I have only been using your awesome JPEGDec on a few other ESP Camera projects. It has been so long I completely forgot but we had a similar discussion there about callbacks. (https://github.com/bitbank2/JPEGDEC/issues/39). I use the pUser a lot, so I think that is why pData confused me at first =)

[bitbank2]

It really sounds like you're overthinking this. Write a simple C++ class and call my C code. keep the PNG structure as a private member variable in your class. Do what you want with the data generated by the library. You don't need to treat it as file operations. Maybe I'm stuck thinking like a C programmer, but what you have been describing sounds like a problem of your own making. You're overthinking the whole issue. Order of operations:

• initialize the structure with the image size info
• Give it scan lines of image data, catch the compressed output
• Close/terminate/finish the process

There really is nothing else to it

[geiseri]

I agree though it's easier to just use just keep them global. Maybe I am stuck thinking like a C++ programmer, I like lifetime control and encapsulation so I tend to go this route first, but hey pick your battles. Either way, your image libraries have yet to disappoint on my esp32 projects, so I am grateful!