langchain4j icon indicating copy to clipboard operation
langchain4j copied to clipboard

Add azure openai audio

Open SandraAhlgrimm opened this issue 1 year ago • 6 comments
trafficstars

Hi! I have started with the audio transcription and would like to get feedback to ensure I'm heading in the right direction, as this is my first PR for this project.

Issue

Closes #255

Change

Add Azure OpenAI

General checklist

  • [X] There are no breaking changes
  • [X] I have added unit and integration tests for my change
  • [X] I have manually run all the unit and integration tests in the module I have added/changed, and they are all green
  • [X] I have manually run all the unit and integration tests in the core and main modules, and they are all green

SandraAhlgrimm avatar Nov 13 '24 21:11 SandraAhlgrimm

/cc @agoncal (azure), @jdubois (azure)

Oh, and one more thing: do we plan to fully support all response_format and timestamp_granularities?

langchain4j avatar Nov 14 '24 15:11 langchain4j

@SandraAhlgrimm any help on this one ?

agoncal avatar Dec 16 '24 13:12 agoncal

@SandraAhlgrimm any help on this one ?

thanks, I put a meeting in your calendar

SandraAhlgrimm avatar Dec 18 '24 12:12 SandraAhlgrimm

When will this be completed?

243006306 avatar Jan 23 '25 07:01 243006306

Is this feature not included in subsequent releases? I think this feature is very useful with ASR

243006306 avatar Apr 17 '25 08:04 243006306

@LizeRaes @langchain4j @dliubarskyi any thoughts :) ?

anunnakian avatar Jun 09 '25 05:06 anunnakian

@anunnakian sorry, thoughts on what exactly? Could you please elaborate?

dliubarskyi avatar Jul 03 '25 10:07 dliubarskyi

@anunnakian sorry, thoughts on what exactly? Could you please elaborate?

Hi @dliubarskyi, I took care of all comments and change requests about this PR (@SandraAhlgrimm gave me access to her branch). Could you please review it 🙏 I'm ready to handle all your feedbacks quickly

anunnakian avatar Jul 04 '25 08:07 anunnakian

Hi @jdubois and @agoncal, I am looking forward to your review

SandraAhlgrimm avatar Aug 20 '25 17:08 SandraAhlgrimm

Spotless is modifying a lot of files that have nothing to do with this PR, I know it will make the CI/CD break but I wouldn't commit those here.

jdubois avatar Aug 21 '25 07:08 jdubois

@jdubois, I took spotless fixes out as discussed and will file this in another PR.

It's ready for your review

SandraAhlgrimm avatar Aug 21 '25 14:08 SandraAhlgrimm

Issue #3576 Pull Request #3577 for formatting

SandraAhlgrimm avatar Aug 21 '25 14:08 SandraAhlgrimm

@anunnakian so sorry about the delay with the review!

dliubarskyi avatar Aug 21 '25 15:08 dliubarskyi

@dliubarskyi thanks for your speedy review! I addressed your comments.

SandraAhlgrimm avatar Aug 21 '25 19:08 SandraAhlgrimm

@SandraAhlgrimm I plan to do a release tomorrow, and this PR is a good candidate to be included. Would you be able to address the comments till tomorrow noon? If not, no worries, I can do it myself, just allow me to push to your repo

dliubarskyi avatar Aug 27 '25 08:08 dliubarskyi

@SandraAhlgrimm please ping me once this will be ready for merge

dliubarskyi avatar Aug 27 '25 16:08 dliubarskyi

@dliubarskyi ready!

SandraAhlgrimm avatar Aug 27 '25 17:08 SandraAhlgrimm

@SandraAhlgrimm only a single concern left: https://github.com/langchain4j/langchain4j/pull/2110#discussion_r2300613557

dliubarskyi avatar Aug 27 '25 18:08 dliubarskyi

@SandraAhlgrimm only a single concern left: #2110 (comment)

Well, we can remove the FileName, but it might cause issues with (Azure) OpenAI for some types and requests

SandraAhlgrimm avatar Aug 27 '25 18:08 SandraAhlgrimm

@SandraAhlgrimm it just does not make sense as far as I understand. Audio should be created from binaryData or base64Data, not from the url. But Audio.getFilename() expects a url.

Since there is no way to pass the file name at the moment, I would remove the getFilename() method and always use "audio.mp3" as the file name in the Azure OpenAI request. WDYT?

dliubarskyi avatar Aug 27 '25 18:08 dliubarskyi

@SandraAhlgrimm it just does not make sense as far as I understand. Audio should be created from binaryData or base64Data, not from the url. But Audio.getFilename() expects a url.

Since there is no way to pass the file name at the moment, I would remove the getFilename() method and always use "audio.mp3" as the file name in the Azure OpenAI request. WDYT?

ok, I am with you! Thanks for explaining extra clear!

SandraAhlgrimm avatar Aug 27 '25 18:08 SandraAhlgrimm

Oh, and one more thing: do we plan to fully support all response_format and timestamp_granularities?

I put this here to finally get the current one merged first :D https://github.com/langchain4j/langchain4j/issues/3604

SandraAhlgrimm avatar Aug 27 '25 19:08 SandraAhlgrimm